On 4/18/2014 6:50 PM, John Coomes wrote:
Alejandro E Murillo (alejandro.muri...@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
