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

[coleen.phillimore at oracle.com]

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/
>