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

[Thomas Stüfe]

src/hotspot/os/windows/os_windows.cpp

Very minor nit: There is the theoretical possibility of _vsnprintf
returning -1 for some reason other than errno. Documentation states "A
return value of -1 indicates that an encoding error has occurred.". Since
it does not say what the state of the output buffer is in that case, it may
have been unchanged, in which case we would return undefined buffer
content. To prevent this, maybe we could set the first buffer byte to \0
before invoking vsnprintf (if len > 0).

However, I admit this is very far fetched. Will probably never happen, at
least, I have never seen it. So, I leave it to you if you do this or not.

---

test/hotspot/gtest/runtime/test_os.cpp

- check_buffer is used to check prefix and suffix range for stray writes? I
think this may be overthinking it a bit, I would not expect strays beyond
buf - 1 and buf + len, in which case you would not need the check_buffer.
- By initializing buffer with \0 you will miss a faulty os::snprintf()
failing to write the terminating zero, no? I would use a different value.

Otherwise, looks good to me.

Best Regards, Thomas

On Sun, Feb 25, 2018 at 11:42 PM, Kim Barrett <kim.barrett at oracle.com>
wrote:

> > On Feb 25, 2018, at 5:37 PM, Kim Barrett <kim.barrett at oracle.com> wrote:
> > Based on discussion, I've changed the new os::vsnprintf and
> > os::snprintf to conform to C99.  […]
> >
> > Also changed jio_vsnprintf to use os::vsnprintf, reverting some of the
> > ealier change.
>
> Just to be clear, the jio_vsnprintf behavior has not been changed.  It’s
> just been
> reimplemented in terms of os::vsnprintf rather than directly using
> ::vsnprintf and
> trying to account for its platform variations.
>
>