[verona.stage] RFR 8087203: Add support for PATCH field and remove unused fields of new version string

[David Holmes]

Hi Alejandro,

I looked at the hotspot and JDK changes.

On 17/06/2015 8:55 AM, Alejandro E Murillo wrote:
>
> Please review these changes:
>
> Bug:  https://bugs.openjdk.java.net/browse/JDK-8087202
> Webrev: http://cr.openjdk.java.net/~amurillo/9/8087202

hotspot/make/Makefile

+ #  VERSION_PATCH Security number for version (e.g. 0)

Comment is wrong.

---

hotspot/src/share/vm/prims/jvm.cpp

3659   info->patch_version = 0;

Why is this hard-wired to zero? Surely this should be a build variable.

---

hotspot/src/share/vm/prims/jvm.h
jdk/src/java.base/share/native/include/jvm.h

These two files should be identical, but now have different comments in 
places.

1188     unsigned int patch_version : 8;
1215     unsigned int patch_version : 8;

In each case you removed two 8 bit fields and added one 8 bit field so 
your bitfields no longer add up to 32 (8+8+16). But these structs seem 
obsolete (due to no hotspot-express) even before the new version 
changes. So I think there is still a bit of follow up work to do here.

---

hotspot/src/share/vm/runtime/java.hpp

110                   _partially_initialized(false),

Isn't the whole partially vs fully initialized issue dead now? Else why 
delete the other code pertaining to that?

---

jdk/src/java.base/share/classes/sun/misc/Version.java.template

   * @since JDK9

Is that the right format? I would have expected @since 9.

  248     private static native boolean getJvmVersionInfo();

Does that method still need to return a boolean ? See next comment

---

jdk/src/java.base/share/native/libjava/Version.c

   50     if (!JDK_InitJvmHandle()) {
   51         JNU_ThrowInternalError(env, "Handle for JVM not found for 
symbol lookup");
   52         return JNI_FALSE;
   53     }
   54     func_p = (GetJvmVersionInfo_fp) 
JDK_FindJvmEntry("JVM_GetVersionInfo");
   55     if (func_p == NULL) {
   56         return JNI_FALSE;
   57     }

this seems dead code now and so the method can't fail or throw an exception.

Thanks,
David
-----

> These are intended to:
>
> (1)  Add support for the patch field of the new version string format
> (2) Remove unused fields remaining from the old version string format
> (3) Patch some jaxp initialization code to be able to handle the new
> version string.
>        this will be pushed under a different bug number, but is required
>        to be able to run and pass all the tests associated with
> "-testset hotspot"
>        (for JFR tests the VM can't be started without that).
>       These will be pushed under this sub-task:
> https://bugs.openjdk.java.net/browse/JDK-8098588
>
> (4) Additional notes:
> (a) Some pieces of code, only necessary for 1.5 or older,  were removed
> (b) update_version and special_update_version were removed
> (c) The code to parse the version string in
> jdk/test/sun/misc/Version/Version.java
>        will probably change when the Version.java class is available.
> (d) As with the changes for 8085822, this is all being pushed to [1] and
> then later to jdk9/dev
> (e) These changes should address most, if not all, of the issues raised
> during
> the code review for JDK-8085822 (see also
> https://bugs.openjdk.java.net/browse/JDK-8087203)
> (f) All  builds and tests pass for "-testset hotspot".
>
> [1] http://hg.openjdk.java.net/verona/stage
>
> Thanks
>