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

[Michal Vala]

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