[8u] RFR: 8196882: VS2017 Hotspot Defined vsnprintf Function Causes C2084 Already Defined Compilation Error

[Kevin Walls]

Hi Kim,

Yes we can try that way.  I can reword the intention to be less 
particular about keeping old windows behaviour.

A new change is that in 8u I found g1GCPhaseTimes.cpp has a vsnprintf 
which was checking for -1 to mean buffer too small.

http://cr.openjdk.java.net/~kevinw/8196882/webrev.02/

Thanks!
Kevin



On 31/07/2018 23:15, Kim Barrett wrote:
>> On Jul 31, 2018, at 5:14 PM, Kevin Walls <kevin.walls at oracle.com> wrote:
>>
>> Great, thanks Kim,
>> Not being an 8 Reviewer is fine, I'm very happy to get your comments here, and when we're happy with it I do a specific 8u approval email.
>>
>> src/os/windows/vm/os_windows.cpp / os::vsnprintf()  oops yes, should not check the result and call _vscprintf().
>> src/share/vm/prims/jvm.cpp / jio_vsnprintf() should not force the null-terminator in the result.
>>
>> Making Windows retain its different behaviour also means the tests need to be different for Windows, so in ostream.cpp  in test_snprintf() I change the boolean passed to the test depending on the platform.  On Windows, expect_count is false.
>>
>> Updated webrev:
>> http://cr.openjdk.java.net/~kevinw/8196882/webrev.01/
>> Thanks
>> Kevin
> I think that trying this hard to retain the different Windows behavior is a mistake.
>
> There aren’t that many callers of vsnprint / os::vsnprintf, and most of them don’t care
> (don’t examine the return value).  We can see all of them, since this is a function inside
> of hotspot, and not something exposed (unlike jio_vsnprintf and friends, which are in
> a “public” header).
>
> We *are* retaining the old behavior for jio_vsnprintf.
>
> One of the few (the only?) other callers that cares is the one in ostream.cpp, and it is
> at least arguable that it has always been somewhat wrong, and was fixed by the
> jdk11 change.
>