[8u] RFR for JDK-8162419: closed/com/oracle/jfr/runtime/TestVMInfoEvent.sh failing after JDK-8155968

[Chris Plummer]

Hi Shafi,

Please update the copyright date to 2016 and change "numbers of" to 
"number of".

I'm not so sure I agree with the comments in the CR that you can just 
backport this change to vsnprintf(), but not the other changes in the 
relevant changeset. For example:

--- a/src/share/vm/runtime/java.cpp    Mon Nov 03 11:34:13 2014 -0800
+++ b/src/share/vm/runtime/java.cpp    Wed Oct 29 10:13:24 2014 +0100
@@ -705,25 +705,35 @@
  }

  void JDK_Version::to_string(char* buffer, size_t buflen) const {
+  assert(buffer && buflen > 0, "call with useful buffer");
    size_t index = 0;
+
    if (!is_valid()) {
      jio_snprintf(buffer, buflen, "%s", "(uninitialized)");
    } else if (is_partially_initialized()) {
      jio_snprintf(buffer, buflen, "%s", "(uninitialized) pre-1.6.0");
    } else {
-    index += jio_snprintf(
+    int rc = jio_snprintf(
          &buffer[index], buflen - index, "%d.%d", _major, _minor);
+    if (rc == -1) return;
+    index += rc;
      if (_micro > 0) {
-      index += jio_snprintf(&buffer[index], buflen - index, ".%d", _micro);
+      rc = jio_snprintf(&buffer[index], buflen - index, ".%d", _micro);
      }

I think your change to vsnprintf() will break JDK_Version::to_string() 
if the above diff if not applied. You could argue that the above code is 
already broken because -1 is could be returned to it on Windows. 
However, your changes expand that risk to all platforms.

cheers,

Chris

On 8/11/16 5:14 AM, Shafi Ahmad wrote:
 > Hi,
 >
 > Could I get one more review for this safe change.
 >
 > Regards,
 > Shafi
 >
 >> -----Original Message-----
 >> From: David Holmes
 >> Sent: Thursday, August 11, 2016 9:52 AM
 >> To: Shafi Ahmad; hotspot-runtime-dev at openjdk.java.net
 >> Subject: Re: [8u] RFR for JDK-8162419:
 >> closed/com/oracle/jfr/runtime/TestVMInfoEvent.sh failing after JDK-
 >> 8155968
 >>
 >> Hi Shafi,
 >>
 >> On 10/08/2016 6:34 PM, Shafi Ahmad wrote:
 >>> Hi,
 >>>
 >>> Please review the code change for "JDK-8162419:
 >> closed/com/oracle/jfr/runtime/TestVMInfoEvent.sh failing after JDK-
 >> 8155968" to jdk8u.
 >>> Please note this is partial backport of
 >> http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/110ec5963eb1#l23.1
 >>> Summary:
 >>> Microsoft version of vsnprintf() behaves differently from the 
standard C
 >> version when there is not enough space in the buffer.
 >>> Microsoft version doesn't null terminates its output under error 
conditions,
 >> whereas the standard C version does. On Windows, it returns -1.
 >>> We handle both cases here and always return -1, and perform null
 >> termination.
 >>
 >> This looks fine to me.
 >>
 >> Thanks,
 >> David
 >>
 >>> Jdk8 bug: https://bugs.openjdk.java.net/browse/JDK-8162419
 >>> Webrev link: http://cr.openjdk.java.net/~shshahma/8162419/webrev.00/
 >>>
 >>> Testing: jprt
 >>>
 >>> Regards,
 >>> Shafi
 >>>