thanks Coleen and Kim!

On 07/30/2018 08:29 PM, coleen.phillim...@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 <mv...@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 <mv...@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

Reply via email to