Re: 6516099: InputStream.skipFully(int k) to skip exactly k bytes

2018-11-16 Thread Brian Burkhalter
Hi Brent, So updated in place. http://cr.openjdk.java.net/~bpb/6516099/webrev.07/ Thanks, Brian > On Nov 16, 2018, at 5:06 PM, Brent Christian > wrote: > > Do we also want an @see in skip(), pointing to skipNBytes() ?

Re: 6516099: InputStream.skipFully(int k) to skip exactly k bytes

2018-11-16 Thread Brent Christian
Do we also want an @see in skip(), pointing to skipNBytes() ? Thanks, -Brent On 11/16/18 11:14 AM, Brian Burkhalter wrote: --- a/src/java.base/share/classes/java/io/InputStream.java +++ b/src/java.base/share/classes/java/io/InputStream.java @@ -579,6 +579,7 @@ * when this

Re: RFR 4947890 : Minimize JNI upcalls in system property initialization

2018-11-16 Thread Mandy Chung
Hi Roger, Looking good.  I have a few small comments: I assume VM.saveAndRemoveProperties will be a separate cleanup. SystemProps::cmdProperties adds the system properties that can skip adding these internal properties to the Properties object. In fact, these internal properties are the

Re: RFR 8177552: Compact Number Formatting support

2018-11-16 Thread naoto . sato
Hi Nishit, Here are my comments: - CLDRConverter: As the compact pattern no more employs List, can we eliminate stringListEntry/Element, and use Array equivalent instead? - CompactNumberFormat.java Multiple locations: Use StringBuilder instead of StringBuffer. line 268: The link points to

Re: RFR (JDK 12/java.base) 8213325: (props) Properties.loadFromXML does not fully comply with the spec

2018-11-16 Thread Joe Wang
Hi Daniel, On 11/16/18, 10:32 AM, Daniel Fuchs wrote: Hi Joe, 1. It looks as if the parser will not complain if the root element is seen twice - and it looks as if it is a supported feature since the previous implementation also had a counter. Should this be tested? If this is a

Re: [OpenJDK 2D-Dev] RFR: 8130264 : change the mechanism by which JDK loads the platform-specific PrinterJob implementation

2018-11-16 Thread Sergey Bylokhov
Looks fine. On 15/11/2018 13:41, Phil Race wrote: bug: http://cr.openjdk.java.net/~prr/8130264/ webrev: http://cr.openjdk.java.net/~prr/8130264/ Currently java launcher code embeds the name of the java.desktop module's PrinterJob implementation class for each platform in a system property

Re: RFR: 8213920 : Use {@systemProperty} tag for properties listed in System.getProperties

2018-11-16 Thread Roger Riggs
Hi Jon, Yes, I skipped the details on how @code is represented to html. The missing piece was that @systemProperty is defined to have the same translation to html as @code. Thanks, Roger On 11/16/2018 03:58 PM, Jonathan Gibbons wrote: Roger, Being pedantic ... javadoc tags don't appear

Re: RFR: 8213920 : Use {@systemProperty} tag for properties listed in System.getProperties

2018-11-16 Thread Jonathan Gibbons
Roger, Being pedantic ... javadoc tags don't appear in stylesheets; HTML elements do. Both {@code} and {@systemProperty} are translated to HTML that uses .  In the case of {@systemProperty} it also contains elements that allow it to be the target for links in the search index and A-Z

Re: RFR: 8213920 : Use {@systemProperty} tag for properties listed in System.getProperties

2018-11-16 Thread Mandy Chung
Looks good. Mandy On 11/15/18 11:00 PM, Priya Lakshmi Muthuswamy wrote: Hi, Kindly review the fix for https://bugs.openjdk.java.net/browse/JDK-8213920 webrev: http://cr.openjdk.java.net/~pmuthuswamy/8213920/webrev.00/ Thanks, Priya

Re: RFR 4947890 : Minimize JNI upcalls in system property initialization

2018-11-16 Thread Erik Joelsson
Thanks, looks good to me now. /Erik On 2018-11-16 12:02, Roger Riggs wrote: Hi Erik, Yes, that is removed. Webrev updated in place. Thanks, Roger http://cr.openjdk.java.net/~rriggs/webrev-props-only-raw/ On 11/16/2018 02:37 PM, Erik Joelsson wrote: Does this mean we can remove

Re: RFR: 8130264 : change the mechanism by which JDK loads the platform-specific PrinterJob implementation

2018-11-16 Thread Roger Riggs
Hi Phil, Looks fine from the core-libs perspective. Thanks, thanks for removing this cross module dependency. Roger On 11/15/2018 04:41 PM, Phil Race wrote: bug: http://cr.openjdk.java.net/~prr/8130264/ webrev: http://cr.openjdk.java.net/~prr/8130264/ Currently java launcher code embeds

Re: RFR: 8213920 : Use {@systemProperty} tag for properties listed in System.getProperties

2018-11-16 Thread Roger Riggs
Hi Jon, Just wanted to know. I know @code has an entry in stylesheet.css, I didn't see where in the stylesheet @systemProperty was covered. Roger On 11/16/2018 03:17 PM, Jonathan Gibbons wrote: Roger, The functionality is inside internal doclet code that handles the {@systemProperty} tag.

Re: RFR: 8213920 : Use {@systemProperty} tag for properties listed in System.getProperties

2018-11-16 Thread Jonathan Gibbons
Roger, The functionality is inside internal doclet code that handles the {@systemProperty} tag. Are you just curious as to how this is working, or are you looking to change the appearance? -- Jon On 11/16/2018 11:59 AM, Roger Riggs wrote: Hi Priya, These changes looks fine. One

Re: RFR 4947890 : Minimize JNI upcalls in system property initialization

2018-11-16 Thread Roger Riggs
Hi Erik, Yes, that is removed. Webrev updated in place. Thanks, Roger http://cr.openjdk.java.net/~rriggs/webrev-props-only-raw/ On 11/16/2018 02:37 PM, Erik Joelsson wrote: Does this mean we can remove $(VERSION_CFLAGS) from System.c in make/lib/CoreLibraries.gmk? Otherwise this looks good

Re: RFR: 8213920 : Use {@systemProperty} tag for properties listed in System.getProperties

2018-11-16 Thread Roger Riggs
Hi Priya, These changes looks fine. One question, since {@systemProperty replaces {@code, where is the control over the appearance of the text?  It still seems to be a code font. Thanks, Roger On 11/16/2018 02:00 AM, Priya Lakshmi Muthuswamy wrote: Hi, Kindly review the fix for

Re: RFR 4947890 : Minimize JNI upcalls in system property initialization

2018-11-16 Thread Erik Joelsson
Does this mean we can remove $(VERSION_CFLAGS) from System.c in make/lib/CoreLibraries.gmk? Otherwise this looks good from a build point of view. /Erik On 2018-11-16 10:49, Roger Riggs wrote: Updates to include the suggestions made by Mandy and Brent:  - Move the build-time properties from

Re: 6516099: InputStream.skipFully(int k) to skip exactly k bytes

2018-11-16 Thread Brian Burkhalter
Great! Now for a CSR. Thanks everyone for all the comments in this protracted thread. Brian > On Nov 16, 2018, at 11:23 AM, Roger Riggs wrote: > > Looks great! > > Thanks, Roger > > > On 11/16/2018 02:14 PM, Brian Burkhalter wrote: >> Hi Roger, >> >> Thanks for the suggestions. Version 7

Re: 6516099: InputStream.skipFully(int k) to skip exactly k bytes

2018-11-16 Thread Roger Riggs
Looks great! Thanks, Roger On 11/16/2018 02:14 PM, Brian Burkhalter wrote: Hi Roger, Thanks for the suggestions. Version 7 http://cr.openjdk.java.net/~bpb/6516099/webrev.07/ is updated in place with this diff: ---

Re: 6516099: InputStream.skipFully(int k) to skip exactly k bytes

2018-11-16 Thread Brian Burkhalter
Hi Roger, Thanks for the suggestions. Version 7 http://cr.openjdk.java.net/~bpb/6516099/webrev.07/ is updated in place with this diff: --- a/src/java.base/share/classes/java/io/InputStream.java +++ b/src/java.base/share/classes/java/io/InputStream.java @@ -579,6 +579,7 @@ *

RFR 4947890 : Minimize JNI upcalls in system property initialization

2018-11-16 Thread Roger Riggs
Updates to include the suggestions made by Mandy and Brent:  - Move the build-time properties from native code to the VersionProps.java.template    including java.vendor.* and java.specification.* properties, plus the java.class.version (major.minor)    This included two changes to the build

Re: FYI: new javadoc tag to document system properties

2018-11-16 Thread Jonathan Gibbons
Roger, In general, and without looking at the JCE API, I note that module-info.java is where providers are declared (i.e. the provides directive and @provides tag.) So this might indeed be a reasonable place to document information relevant to the provider, such as any properties it might

Re: RFR (JDK 12/java.base) 8213325: (props) Properties.loadFromXML does not fully comply with the spec

2018-11-16 Thread Daniel Fuchs
Hi Joe, 1. It looks as if the parser will not complain if the root element is seen twice - and it looks as if it is a supported feature since the previous implementation also had a counter. Should this be tested? If this is a feature and multiple roots are supported then the new

Re: RFR: 8148188: Enhance the security libraries to record events of interest

2018-11-16 Thread Sean Mullan
Updated webrev looks good. --Sean On 11/16/18 10:31 AM, Seán Coffey wrote: I've renamed the 'peerCertificateId' variable and label in TLSHandshakeEvent to 'certificateId'. This allows the event data to be co-displayed in JMC views which share the same type of data (@CertificateId). I've

RFR 8177552: Compact Number Formatting support

2018-11-16 Thread Nishit Jain
Hi, Please review this non trivial feature addition to NumberFormat API. The existing NumberFormat API provides locale based support for formatting and parsing numbers which includes formatting decimal, percent, currency etc, but the support for formatting a number into a human readable or

Re: adding rsockets support into JDK

2018-11-16 Thread Daniel Fuchs
Hi Lucy, I'm assuming that the classes in questions should eventually have javadoc generated for them in the as a JDK implementation-specific supported API, yes? If so you might want to try and figure out how the javadoc generation of the jdk.httpserver modules is hooked up in the build system

Re: 6516099: InputStream.skipFully(int k) to skip exactly k bytes

2018-11-16 Thread Brian Burkhalter
Hi Roger, I can add these things. The more recent version is http://cr.openjdk.java.net/~bpb/6516099/webrev.07/ With these changes do you think we’ve converged on a solution? Thanks, Brian > On Nov 15, 2018, at 8:23 AM, Roger Riggs wrote: > > Looking good. > > I would add a message to the

Re: RFR: 8148188: Enhance the security libraries to record events of interest

2018-11-16 Thread Seán Coffey
I've renamed the 'peerCertificateId' variable and label in TLSHandshakeEvent to 'certificateId'. This allows the event data to be co-displayed in JMC views which share the same type of data (@CertificateId). I've uploaded an example screenshot [1] I've also made an adjustment to the

Re: FYI: new javadoc tag to document system properties

2018-11-16 Thread Roger Riggs
Hi, module-info.java isn't a good place; its too far from the relevant API that it affects. The property should be documented where it make sense and where developers will find it. In this case, since it is provider specific,  it makes sense to define the property where the provider is