RFR: JDK-8208084: Windows build failure - "'snprintf': identifier not found"

[Michal Vala]

thanks Coleen and Kim!

On 07/30/2018 08:29 PM, coleen.phillimore at oracle.com wrote:
> 
> This looks good.    I will sponsor it.
> Coleen
> 
> On 7/30/18 8:00 AM, Michal Vala wrote:
>>
>>
>> On 07/27/2018 06:22 PM, Kim Barrett wrote:
>>>> On Jul 24, 2018, at 2:03 AM, Michal Vala <mvala at redhat.com> wrote:
>>>>
>>>>
>>>>
>>>> On 07/23/2018 07:15 PM, Erik Joelsson wrote:
>>>>> Hello,
>>>>> On 2018-07-23 08:27, Kim Barrett wrote:
>>>>>>> On Jul 23, 2018, at 9:38 AM, Michal Vala <mvala at redhat.com> wrote:
>>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> JDK-8208084 introduced build failure on Windows, where `snprintf` 
>>>>>>> function is not implemented by Visual Studio 2013 (currently latest 
>>>>>>> compiler supported by OpenJDK).
>>>>>>>
>>>>>>> I believe using `sprintf` is safe here. Please see the webrev. If it's 
>>>>>>> ok, I would also need a sponsor for this.
>>>>>>>
>>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8208084
>>>>>>> webrev: http://cr.openjdk.java.net/~mvala/jdk/jdk/JDK-8208084/webrev.00/
>>>>>>>
>>>>>>>
>>>>>>> Thanks
>>>>>>>
>>>>>>> -- 
>>>>>>> Michal Vala
>>>>>>> OpenJDK QE
>>>>>>> Red Hat Czech
>>>>>> It seems there's some documentation that needs to be updated.  JDK 11
>>>>>> supports building with VS2017, and I think with JDK 12 we'll be
>>>>>> requiring it, in preparation for starting to use features from more
>>>>>> recent C++ standards.
>>>>>>
>>>>> Indeed, that's on me. I just sent out an RFR to correct this.
>>>>
>>>> Your changeset doesn't update minimal Visual Studio version. It's not 
>>>> buildable with VS2010.
>>>
>>> If it’s really necessary to be able to build JDK 12 with earlier versions of 
>>> Visual Studio for now
>>> (as I said, it might become impossible later for other reasons), the proper 
>>> fix is to replace the
>>> call to snprintf with os::snprintf, which is what should have been used in 
>>> the first place.  I missed
>>> that when I was reviewing JDK-8207359.
>>>
>>>
>>
>> You're right. That also solves the issue with VS2013 and is imho proper way to 
>> implement it anyway. Here's the new webrev: 
>> http://cr.openjdk.java.net/~mvala/jdk/jdk/JDK-8208084/webrev.01/
>>
> 

-- 
Michal Vala
OpenJDK QE
Red Hat Czech