RFR: 8072379: Implement jdk.Version and jdk.OracleVersion

Iris Clark iris.clark at oracle.com
Thu Jan 14 05:27:04 UTC 2016


Hi, Mandy.

Thanks for looking at this webrev again.

> 273             current = parse(System.getProperty("java.version"));
> 
> System.getProperty will do a permission check and it needs to be called within a doPrivileged block.

Nice catch!  I've added the permission check and the associated @throws.

> 154  * @see  <a href="http://openjdk.java.net/jeps/223">JEP 223: New Version-String Scheme</a>
> 
> Does the javadoc have the essential information from this JEP?  Wonder if this @see is necessary.

I thought it might be nice to include the JEP reference as it contains additional
data about versioning beyond the JavaDoc for this method; however, it's not
critical to the understanding or use of this class.

For some reason I'd thought that there was precedence in including JEP numbers
as @see references, but I find no evidence of that.  I've removed the @see.

These are the diffs to address both of your comments:

$ diff Version.java_save Version.java
28a29,30
> import java.security.AccessController;
> import java.security.PrivilegedAction;
154,155d155
<  * @see  <a href="http://openjdk.java.net/jeps/223">JEP 223: New Version-String Scheme</a>
<  *
268a269,274
>      * @throws  SecurityException
>      *          If a security manager exists and its {@link
>      *          SecurityManager#checkPropertyAccess(String)
>      *          checkPropertyAccess} method does not allow access to the
>      *          system property "java.version"
>      *
272,273c278,285
<       if (current == null)
<           current = parse(System.getProperty("java.version"));
---
>       if (current == null) {
>           current = parse(AccessController.doPrivileged(
>                 new PrivilegedAction<>() {
>                     public String run() {
>                         return System.getProperty("java.version");
>                     }
>                 }));
>       }

Regards,
iris

-----Original Message-----
From: Mandy Chung 
Sent: Tuesday, January 12, 2016 2:26 PM
To: Iris Clark
Cc: Mandy Chung; Joe Darcy; Magnus Ihse Bursie; Roger Riggs; Alan Bateman; core-libs-dev; verona-dev at openjdk.java.net
Subject: Re: RFR: 8072379: Implement jdk.Version and jdk.OracleVersion


> On Jan 11, 2016, at 1:44 PM, Iris Clark <iris.clark at oracle.com> wrote:
> 
> Hi, Joe, Roger, Alan, Magnus, and Mandy.
> 
> At the end of December (shortly before the Christmas/Winter break and 
> my vacation), I provided responses to your messages and an updated 
> webrev:
> 
>  http://cr.openjdk.java.net/~iris/verona/8072379/webrev.2/

It’s good to see that jdk.OracleVersion has been removed and use jdk.Version to obtain the fourth element of the version number.

This patch looks good in general.  Minor comment:

 273             current = parse(System.getProperty("java.version"));

System.getProperty will do a permission check and it needs to be called within a doPrivileged block.

 154  * @see  <a href="http://openjdk.java.net/jeps/223">JEP 223: New Version-String Scheme</a>

Does the javadoc have the essential information from this JEP?  Wonder if this @see is necessary.

Alan already comment on the “jdk” package that needs to find the proper module to export it (that’s a future RFE) and modules.xml should be updated.

Mandy



More information about the core-libs-dev mailing list