RFR: 8196882: VS2017 Hotspot Defined vsnprintf Function Causes C2084 Already Defined Compilation Error

[Kim Barrett]

> On Feb 21, 2018, at 3:23 AM, Marcus Larsson <marcus.larsson at oracle.com> wrote:
> 
> Hi,
> 
> 
> On 2018-02-21 08:30, Thomas Stüfe wrote:
>> Hi Kim,
>> 
>> this is good. Please find comments inline / below.
>> 
>> On Wed, Feb 21, 2018 at 1:08 AM, Kim Barrett <kim.barrett at oracle.com> wrote:
>>> We still provide os::log_vsnprintf, which differs from the new
>>> os::vsnprintf in the return value when output truncation occurs. […]
>>> 
>> I totally agree. Possible alternatives:
>> 
>> 1 rename it
>> 2 move it into the log sub project as an internal implementation detail
>> 3 Or, provide a platform independent version of _vcsprintf (
>> https://msdn.microsoft.com/en-us/library/w05tbk72.aspx) instead. So whoever
>> really wants to count characters in resolved format string should first use
>> that function, then alloc the appropiate buffer, then do the real printing.
>> Posix variant for  _vcsprintf could just be vsnprintf with a zero byte
>> output buffer.
>> 
>> I personally like (3) best, followed by (2)
> 
> The best alternative, IMHO, would be to make os::vsnprintf behave just like log_vsnprintf (C99 standard vsnprintf), thus removing the need for log_vsnprintf completely. Also, that behavior is strictly better than always returning -1 on error.

There was discussion of the behavior in this area around JDK-8062370
http://mail.openjdk.java.net/pipermail/hotspot-dev/2014-November/015779.html

The consensus then was that a guarantee of NUL-termination was
desirable. That's already at variance with C99 vsnprintf.

It was noted at the time that most calls to these functions in HotSpot
ignored the return value. That's still true. I looked at the 57 I
found via egrep "[=,(]\s*(jio_|::|)v?snprintf\(", and only found 2
that seem to want the C99 vsnprintf return value behavior. (Also found
a handful that appear to be mishandling the result in one way or
another; will be filing some more bugs...) I didn't look at the
roughly 350 calls that are ignoring the result.

So being consistent with C99 vsnprintf doesn't appear to be useful in
this code base, would add some cost to converting away from using the
jio_ functions in order to get the -Wformat warnings, and might add
some (likely not really measurable) performance cost for Windows.

So I'm not inclined toward making the return value from os::vsnprintf
consistent with C99 vsnprintf. If there's a groundswell of support, it
can be done, but I'd rather keep such an effort separate from this
change, which is intended to address a problem with building using
recent versions of Visual Studio, doing a little bit of cleanup in the
process.