RFR 8030011: Update Hotspot version string output

[Alejandro E Murillo]

On 4/18/2014 6:50 PM, John Coomes wrote:
> Alejandro E Murillo (alejandro.murillo at oracle.com) wrote:
>> Please review this change to make the hotspot related output produced by
>> "java -version"
>> match the corresponding JDK output:
>>
>> webrev: http://cr.openjdk.java.net/~amurillo/9/8030011/
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8030011
>>
>> Note that we initially wanted to obtain more information from the repo
>> being built and add
>> it to the hotspot version output, but that will require changes in the
>> JPRT, so
>> we have decided to split the change in 2 bugs. One to make the version match
>> (above webrev) and another one, an RFE, to add additional information
>> extracted from the repo.
> Generally looks good, but I agree with David that the long lines
> should be wrapped, and it might even be clearer to separate them into
> two variables, e.g.,
>
> JDK_VERSION_DEFS = -DJDK_MAJOR_VERSION="\"$(JDK_MAJOR_VERSION)\"" \
> 		   -DJDK_MINOR_VERSION="\"$(JDK_MINOR_VERSION)\"" \
> 		   ... other defs ...
> VERSION_DEFS  = -DHOTSPOT_RELEASE_VERSION="\"$(HS_BUILD_VER)\"" \
> 		-DJRE_RELEASE_VERSION="\"$(JRE_RELEASE_VER)\"" \
> 		$(JDK_VERSION_DEFS)
>
> Also the change to this part of vm.make differs between at least the
> solaris version and the linux version (didn't check bsd or windows).
> Please make them all consistent.
>
>> Note that in the current version of vm_version.cpp, there is no error
>> checking in product mode,
>> I was suggested to make that explicit.
> I like the additional error checking.  I think you could eliminate the
> 4 copies of similar code with a function that does the checks and sets
> the field, e.g.,
>
>      void set_version_field(int * version_field, const char * version_str,
> 			   const char * const assert_msg) {
>         if (version_str != NULL ...) {
> 	 DEBUG_ONLY(assert_digits(version_str, assert_msg));
> 	 *version_field = atoi(version_str);
>         }
>      }
>
> -John
Thanks John,
working on adding these changes and sanity testing

Thanks

-- 
Alejandro