RFR: 8196882: VS2017 Hotspot Defined vsnprintf Function Causes C2084 Already Defined Compilation Error
Kim Barrett
kim.barrett at oracle.com
Wed Feb 21 00:08:56 UTC 2018
Please review this change to how HotSpot handles platform variations
of vsnprintf and snprintf.
We propose to provide os::vsnprintf and os::snprintf, which have the
agreed behavior for such functions for use in HotSpot; see
jio_vsnprintf and friends. Specifically, (1) always NUL-terminate the
target buffer unless its indicated size is zero, and (2) return -1 on
output truncation. This places the platform dependent code in
OS-specific files where it belongs (os_windows.cpp and os_posix.cpp),
while giving the rest of HotSpot a consistent API to use.
An additional benefit is that these new os functions are decorated
with format attributes, so will produce warnings for incorrect calls
(on platforms which support the decorations and associated
checking). The jio_ variants have the attributes in jvm.cpp rather
than jvm.h, making them largely useless. (Maybe that's just a bug? But
jvm.h doesn't have the infrastructure for platform-dependent
configuration that is available for in-HotSpot code.) However, to
really get the benefit of this, we will need to change HotSpot code to
consistently use the os functions, rather than the jio_ equivalents.
(That would have the additional benefit of not needing to include
jvm.h all over the place just to have access to the jio_ functions.)
That's a change for later, not part of this change.
We still provide os::log_vsnprintf, which differs from the new
os::vsnprintf in the return value when output truncation occurs. (It
returns the size the output would have been had it not been
truncated.) It has been changed to always NUL-terminate the output,
and documented as such. None of the current uses care, and this makes
it consistent with os::vsnprintf. Note that os::log_vsnprintf was
added as part of UL and is presently only used by it. Maybe it could
have a better name?
This change leaves no direct calls to vsnprintf in HotSpot, outside of
the relevant parts of the os implementation. However, there are a lot
of direct calls to snprintf, potentially resulting in strings that are
not NUL-terminated. Those should perhaps be calling os::snprintf (or
previously, jio_snprintf), but that's another change for later.
CR:
https://bugs.openjdk.java.net/browse/JDK-8196882
Webrev:
http://cr.openjdk.java.net/~kbarrett/8196882/open.00/
Testing:
Mach5 {hs,jdk}-tier{1,2,3} (used VS2013 for Windows testing)
Also built with VS2017.
More information about the hotspot-dev
mailing list