RE: RFR: 8189102: All tools should support -?, -h and --help

[Lindenmaier, Goetz]

OK, thanks!

Best regards,
  Goetz.

> -Original Message-
> From: Weijun Wang [mailto:weijun.w...@oracle.com]
> Sent: Wednesday, November 22, 2017 8:08 AM
> To: Lindenmaier, Goetz 
> Cc: core-libs-dev@openjdk.java.net; compiler-...@openjdk.java.net;
> serviceability-dev (serviceability-...@openjdk.java.net)  d...@openjdk.java.net>
> Subject: Re: RFR: 8189102: All tools should support -?, -h and --help
> 
> Hi Goetz
> 
> I would just remove it.
> 
> Thanks
> Max
> 
> > On Nov 22, 2017, at 2:53 PM, Lindenmaier, Goetz
>  wrote:
> >
> > Hi Max,
> >
> > while removing the comments from the k-tools, I saw this:
> >
> > ---
> a/src/java.security.jgss/windows/classes/sun/security/krb5/internal/tools/Kl
> ist.java  Tue Oct 10 14:39:45 2017 +0200
> > +++
> b/src/java.security.jgss/windows/classes/sun/security/krb5/internal/tools/Kl
> ist.java  Tue Nov 21 13:09:17 2017 +0100
> > @@ -356,7 +358,6 @@
> > System.out.println("\t-t \t shows keytab entry timestamps");
> > System.out.println("\t-K \t shows keytab entry key value");
> > System.out.println("\t-e \t shows keytab entry key type");
> > -System.out.println("\nUsage: java sun.security.krb5.tools.Klist " +
> > -   "-help for help.");
> > +System.out.println("\n-? -h --help print this help message and 
> > exit");
> > }
> > }
> >
> > I don't think the old comment is in the original program :) and anyways, -
> help
> > is not supported by Klist.  It prints the usage called with the flag, but 
> > just
> because
> > it prints it on any wrong flag.
> >
> > So should I replace the comment here?
> > Or at least remove it?
> >
> > Best regards,
> >  Goetz
> >
> >> -Original Message-
> >> From: Weijun Wang [mailto:weijun.w...@oracle.com]
> >> Sent: Monday, November 20, 2017 3:49 PM
> >> To: Lindenmaier, Goetz 
> >> Cc: core-libs-dev@openjdk.java.net; compiler-...@openjdk.java.net;
> >> serviceability-dev (serviceability-...@openjdk.java.net)  >> d...@openjdk.java.net>
> >> Subject: Re: RFR: 8189102: All tools should support -?, -h and --help
> >>
> >> Hi Goetz
> >>
> >> I understand your intention.
> >>
> >> If the reason is that one day every tool will switch to this new style, 
> >> please
> at
> >> least do not include kinit, klist and ktab. These Windows-only commands
> are
> >> meant to emulate MIT krb5 tools with the same names and we need to
> keep
> >> the option (and maybe the help screen) as similar as possible.
> >>
> >> I am OK with supporting the --help option undocumented.
> >>
> >> Thanks
> >> Max
> >>
> >>> On Nov 20, 2017, at 9:46 PM, Lindenmaier, Goetz
> >>  wrote:
> >>>
> >>> Hi Max,
> >>>
> >>> I think there are so many tools mixing both kinds of
> >>> options, that it's rather a gain if all at least document
> >>> the same, up to date help message, than if the documentation
> >>> is skipped for some.
> >>>
> >>> After my change, all the help messages are quite similar.
> >>> I updated them to use the same wording while trying to
> >>> keep the sentence structure in accordance with the other
> >>> documented flags, see below.
> >>>
> >>> Best regards,
> >>> Goetz.
> >>>
> >>>
> >>>
> >>> -? -h --help   display this help message
> >>> -? -h --help  display this help message
> >>> -h, --help (Print this help message.)
> >>> -?, --help   Print this help message
> >>> -? -h --help Print this help message
> >>> --help, -h, -?  Display command line options and exit
> >>> -? -h --help Print this help message
> >>> -h -? --help  Print this help message
> >>> -? -h --help
> >>> -? -h --help  : display this help message and exit
> >>> -? -h --help   :  display this help message and exit
> >>> -? -h --help  print this help message and exit
> >>> -? | -h | --help to print this help message
> >>> jmap -? -h --help  to print this help message
> >>> usage: jps [-? -h --help]
> >>> -? -h --help to print this help message
> >>> -? -h --help  Prints this help message.
> >>> -? -h --help print this help message
> >>> -? -h --help  Print this synopsis of standard options and exit
> >>> Use "keytool -? -h or --help" for all available commands
> >>> --helpprint this help message to the output stream
> >>> -?, -h, --help   Print this help message
> >>> -h, --help, -?Print this help message
> >>> -?, -h, --help   Print this help message
> >>> -? -h --help Print this help message and exit
> >>> -?, -h, --help  print this help message
> >>> -?, -h, --helpprint this help message
> >>> -? -h --help   Print this help message
> >>> -?, -h, --help[:compat]Give this, or optionally the compatibility, 
> >>> help
> >>> [-? -h --help]  

Re: RFR: 8187954 Update JAX-WS RI integration to latest version

[Jack Li]

Hi,

any response?

> On Nov 15, 2017, at 08:05, Jack Li  wrote:
> 
> Hi Lance
> 
> Is there any other issue about it? can you approve it to merge?
> 
>> On Nov 6, 2017, at 07:59, ZhengJun Li > > wrote:
>> 
>> 
>> Yes, all the tests are passed.
>> 
>> 
>> 在 2017年11月6日，05:05，Lance Andersen > > 写道：
>> 
>>> Hi Jack,
>>> 
>>> Overall looks OK.  I am assuming all of the test suites are passing?
>>> 
>>> Best
>>> Lance
 On Nov 2, 2017, at 7:34 AM, Lance Andersen > wrote:
 
 Hi Jack
 
 Its on my list to finish by the end of the week. 
 
 Best
 Lance
> On Nov 2, 2017, at 4:34 AM, Jack Li  > wrote:
> 
> Hi Lance
> 
> Is there anything wrong in the new webrev?
> 
> 
>> On Oct 25, 2017, at 10:00, Jack Li >  > >> wrote:
>> 
>> Hi Lance,
>> 
>> The webrev is updated, can you please review it again?
>> 
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8187954 
>>  
>> > > 
>> Webrev: 
>> http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8187954/10/01 
>>  
>> > >
>> 
>> Summary of changes:
>> 
>> jaxws/src/java.xml.bind/share/classes/javax/xml/bind/*
>> JDK-8186946 - Fix accessibility and other issues in the java.xml.bind 
>> module
>> 
>> jaxws/src/java.xml.ws/share/classes/com/sun/xml/internal/messaging/saaj/**
>> JDK-8186314 - code at c.s.x.i.m.saaj.soap.MessageImpl must be modified 
>> to avoid crash after javac change
>> And also contains the fixes for importing nodes for SOAPDocumentFragment
>> 
>> 
>> Patch also contains several small bugfixes, not tracked in JBS.
>> 
>>> On Oct 11, 2017, at 18:47, Lance Andersen >>  >> >> wrote:
>>> 
>>> Hi Jack,
>>> 
>>> I would prefer to see an updated webrev so that we do not inadvertently
>>> push these changes.
>>> 
>>> Best
>>> Lance
 On Oct 11, 2017, at 3:26 AM, Jack Li  >> wrote:
 
 Hi Lance
 
 I will update them in Metro repository, do I need to regenerate webrev?
 or can you skip the files this time and I fix it in next integration?
 
> On Oct 9, 2017, at 19:35, Lance Andersen    >> wrote:
> 
> Hi Jack,
> 
> UnMarshaller also has the same issue.  I would update the webrev 
> given the number of places to help sanity check for omissions
> 
> Best
> Lance
>> On Oct 8, 2017, at 9:22 PM, Jack Li >  > >> wrote:
>> 
>> Hi Lance,
>> 
>> the change is incorrect, it should be “javax/xml/bind”.
>> thanks a lot for your finding, do you think I need to fix it and 
>> resubmit the webrev this time?
>> or can you skip this file this time and I fix it in next integration?
>> 
>>> On Oct 4, 2017, at 02:09, Lance Andersen >>  
>>> >> >> wrote:
>>> 
>>> Hi Jack,
>>> 
>>> Is this change correct:
>>> 
>>> -
>>> --- 
>>> old/src/java.xml.bind/share/classes/javax/xml/bind/Marshaller.java  
>>> 2017-09-29 13:58:31.968185273 +0100
>>> +++ 
>>> new/src/java.xml.bind/share/classes/javax/xml/bind/Marshaller.java  
>>> 2017-09-29 13:58:31.676185267 +0100
>>> @@ -373,7 +373,7 @@
>>>  *  If the {@link ValidationEventHandler 
>>> ValidationEventHandler}

Re: RFR 8191173 : (cl) Clarify or remove "for delegation" in ClassLoader spec

[Alan Bateman]

On 21/11/2017 20:41, Brent Christian wrote:

Hi,

Please review my change to this small bit of ClassLoader spec that can 
be tidied up, as noticed by Martin.


Issue:
https://bugs.openjdk.java.net/browse/JDK-8191173
Webrev:
http://cr.openjdk.java.net/~bchristi/8191173/webrev.00/
This looks okay. One could argue that getParent should be changed too as 
there is never any guarantee that delegation will be to the parent but 
what you have is fine.


-Alan

Re: RFR: 8189102: All tools should support -?, -h and --help

[Weijun Wang]

Hi Goetz

I would just remove it.

Thanks
Max

> On Nov 22, 2017, at 2:53 PM, Lindenmaier, Goetz  
> wrote:
> 
> Hi Max, 
> 
> while removing the comments from the k-tools, I saw this:
> 
> --- 
> a/src/java.security.jgss/windows/classes/sun/security/krb5/internal/tools/Klist.java
>   Tue Oct 10 14:39:45 2017 +0200
> +++ 
> b/src/java.security.jgss/windows/classes/sun/security/krb5/internal/tools/Klist.java
>   Tue Nov 21 13:09:17 2017 +0100
> @@ -356,7 +358,6 @@
> System.out.println("\t-t \t shows keytab entry timestamps");
> System.out.println("\t-K \t shows keytab entry key value");
> System.out.println("\t-e \t shows keytab entry key type");
> -System.out.println("\nUsage: java sun.security.krb5.tools.Klist " +
> -   "-help for help.");
> +System.out.println("\n-? -h --help print this help message and 
> exit");
> }
> }
> 
> I don't think the old comment is in the original program :) and anyways, -help
> is not supported by Klist.  It prints the usage called with the flag, but 
> just because
> it prints it on any wrong flag.
> 
> So should I replace the comment here?
> Or at least remove it?
> 
> Best regards,
>  Goetz
> 
>> -Original Message-
>> From: Weijun Wang [mailto:weijun.w...@oracle.com]
>> Sent: Monday, November 20, 2017 3:49 PM
>> To: Lindenmaier, Goetz 
>> Cc: core-libs-dev@openjdk.java.net; compiler-...@openjdk.java.net;
>> serviceability-dev (serviceability-...@openjdk.java.net) > d...@openjdk.java.net>
>> Subject: Re: RFR: 8189102: All tools should support -?, -h and --help
>> 
>> Hi Goetz
>> 
>> I understand your intention.
>> 
>> If the reason is that one day every tool will switch to this new style, 
>> please at
>> least do not include kinit, klist and ktab. These Windows-only commands are
>> meant to emulate MIT krb5 tools with the same names and we need to keep
>> the option (and maybe the help screen) as similar as possible.
>> 
>> I am OK with supporting the --help option undocumented.
>> 
>> Thanks
>> Max
>> 
>>> On Nov 20, 2017, at 9:46 PM, Lindenmaier, Goetz
>>  wrote:
>>> 
>>> Hi Max,
>>> 
>>> I think there are so many tools mixing both kinds of
>>> options, that it's rather a gain if all at least document
>>> the same, up to date help message, than if the documentation
>>> is skipped for some.
>>> 
>>> After my change, all the help messages are quite similar.
>>> I updated them to use the same wording while trying to
>>> keep the sentence structure in accordance with the other
>>> documented flags, see below.
>>> 
>>> Best regards,
>>> Goetz.
>>> 
>>> 
>>> 
>>> -? -h --help   display this help message
>>> -? -h --help  display this help message
>>> -h, --help (Print this help message.)
>>> -?, --help   Print this help message
>>> -? -h --help Print this help message
>>> --help, -h, -?  Display command line options and exit
>>> -? -h --help Print this help message
>>> -h -? --help  Print this help message
>>> -? -h --help
>>> -? -h --help  : display this help message and exit
>>> -? -h --help   :  display this help message and exit
>>> -? -h --help  print this help message and exit
>>> -? | -h | --help to print this help message
>>> jmap -? -h --help  to print this help message
>>> usage: jps [-? -h --help]
>>> -? -h --help to print this help message
>>> -? -h --help  Prints this help message.
>>> -? -h --help print this help message
>>> -? -h --help  Print this synopsis of standard options and exit
>>> Use "keytool -? -h or --help" for all available commands
>>> --helpprint this help message to the output stream
>>> -?, -h, --help   Print this help message
>>> -h, --help, -?Print this help message
>>> -?, -h, --help   Print this help message
>>> -? -h --help Print this help message and exit
>>> -?, -h, --help  print this help message
>>> -?, -h, --helpprint this help message
>>> -? -h --help   Print this help message
>>> -?, -h, --help[:compat]Give this, or optionally the compatibility, help
>>> [-? -h --help]  Print this help message
>>> jstatd -?|-h|--help
>>> 
>>> 
 -Original Message-
 From: Weijun Wang [mailto:weijun.w...@oracle.com]
 Sent: Sonntag, 19. November 2017 01:28
 To: Lindenmaier, Goetz 
 Cc: core-libs-dev@openjdk.java.net; compiler-...@openjdk.java.net;
 serviceability-dev (serviceability-...@openjdk.java.net) >>> d...@openjdk.java.net>
 Subject: Re: RFR: 8189102: All tools should support -?, -h and --help
 
 I am OK with all commands supporting --help, but I am not sure if every
>> tool
 should show it on the help screen if the tools's other options are still 
 using

RE: RFR: 8189102: All tools should support -?, -h and --help

[Lindenmaier, Goetz]

Hi Max, 

while removing the comments from the k-tools, I saw this:

--- 
a/src/java.security.jgss/windows/classes/sun/security/krb5/internal/tools/Klist.java
Tue Oct 10 14:39:45 2017 +0200
+++ 
b/src/java.security.jgss/windows/classes/sun/security/krb5/internal/tools/Klist.java
Tue Nov 21 13:09:17 2017 +0100
@@ -356,7 +358,6 @@
 System.out.println("\t-t \t shows keytab entry timestamps");
 System.out.println("\t-K \t shows keytab entry key value");
 System.out.println("\t-e \t shows keytab entry key type");
-System.out.println("\nUsage: java sun.security.krb5.tools.Klist " +
-   "-help for help.");
+System.out.println("\n-? -h --help print this help message and exit");
 }
 }

I don't think the old comment is in the original program :) and anyways, -help
is not supported by Klist.  It prints the usage called with the flag, but just 
because
it prints it on any wrong flag.

So should I replace the comment here?
Or at least remove it?

Best regards,
  Goetz

> -Original Message-
> From: Weijun Wang [mailto:weijun.w...@oracle.com]
> Sent: Monday, November 20, 2017 3:49 PM
> To: Lindenmaier, Goetz 
> Cc: core-libs-dev@openjdk.java.net; compiler-...@openjdk.java.net;
> serviceability-dev (serviceability-...@openjdk.java.net)  d...@openjdk.java.net>
> Subject: Re: RFR: 8189102: All tools should support -?, -h and --help
> 
> Hi Goetz
> 
> I understand your intention.
> 
> If the reason is that one day every tool will switch to this new style, 
> please at
> least do not include kinit, klist and ktab. These Windows-only commands are
> meant to emulate MIT krb5 tools with the same names and we need to keep
> the option (and maybe the help screen) as similar as possible.
> 
> I am OK with supporting the --help option undocumented.
> 
> Thanks
> Max
> 
> > On Nov 20, 2017, at 9:46 PM, Lindenmaier, Goetz
>  wrote:
> >
> > Hi Max,
> >
> > I think there are so many tools mixing both kinds of
> > options, that it's rather a gain if all at least document
> > the same, up to date help message, than if the documentation
> > is skipped for some.
> >
> > After my change, all the help messages are quite similar.
> > I updated them to use the same wording while trying to
> > keep the sentence structure in accordance with the other
> > documented flags, see below.
> >
> > Best regards,
> >  Goetz.
> >
> >
> >
> > -? -h --help   display this help message
> > -? -h --help  display this help message
> > -h, --help (Print this help message.)
> > -?, --help   Print this help message
> > -? -h --help Print this help message
> > --help, -h, -?  Display command line options and exit
> > -? -h --help Print this help message
> > -h -? --help  Print this help message
> > -? -h --help
> > -? -h --help  : display this help message and exit
> > -? -h --help   :  display this help message and exit
> > -? -h --help  print this help message and exit
> > -? | -h | --help to print this help message
> > jmap -? -h --help  to print this help message
> > usage: jps [-? -h --help]
> > -? -h --help to print this help message
> > -? -h --help  Prints this help message.
> > -? -h --help print this help message
> > -? -h --help  Print this synopsis of standard options and exit
> > Use "keytool -? -h or --help" for all available commands
> > --helpprint this help message to the output stream
> > -?, -h, --help   Print this help message
> > -h, --help, -?Print this help message
> > -?, -h, --help   Print this help message
> > -? -h --help Print this help message and exit
> > -?, -h, --help  print this help message
> > -?, -h, --helpprint this help message
> > -? -h --help   Print this help message
> > -?, -h, --help[:compat]Give this, or optionally the compatibility, help
> > [-? -h --help]  Print this help message
> > jstatd -?|-h|--help
> >
> >
> >> -Original Message-
> >> From: Weijun Wang [mailto:weijun.w...@oracle.com]
> >> Sent: Sonntag, 19. November 2017 01:28
> >> To: Lindenmaier, Goetz 
> >> Cc: core-libs-dev@openjdk.java.net; compiler-...@openjdk.java.net;
> >> serviceability-dev (serviceability-...@openjdk.java.net)  >> d...@openjdk.java.net>
> >> Subject: Re: RFR: 8189102: All tools should support -?, -h and --help
> >>
> >> I am OK with all commands supporting --help, but I am not sure if every
> tool
> >> should show it on the help screen if the tools's other options are still 
> >> using
> >> the old single-"-" style. It looks like the tools are half-converted.
> >>
> >> Is there a timetable to add "--" support to all tools?
> >>
> >> Thanks
> >> Max
> >>
> >>> On Nov 17, 2017, at 7:23 PM, Lindenmaier, Goetz
> 

Re: Incorrect validation of DST in java.util.SimpleTimeZone

[Venkateswara R Chintala]

Thanks Peter for sponsoring this patch. Is there anything else that 
needs to be done from my end for this patch to be integrated? Please let 
me know.


-Venkat


On 14/11/17 8:46 PM, Peter Levart wrote:

Hi Venkat,

I created the following issue:

https://bugs.openjdk.java.net/browse/JDK-8191216

I can also sponsor this patch and push it for JDK 10.

The patch that you are proposing looks good to me. Does anybody have 
anything else to say?


Regards, Peter


On 11/13/2017 11:28 AM, Venkateswara R Chintala wrote:
Thanks David, Peter for your review and comments. As I am new to the 
community, can you please help me to open a bug and integrate the 
changes into code base?


-Venkat

On 12/11/17 2:19 AM, Peter Levart wrote:

Hi David, Venkat,

On 11/11/17 21:15, Peter Levart wrote:

For example, take the following method:

String defaultTZID() {
    return TimeZone.getDefault().getID();
}

When JIT compiles it and inlines invocations to other methods 
within it, it can prove that cloned TimeZone instance never escapes 
the call to defaultTZID() and can therefore skip allocating the 
instance on heap.


But this is fragile. If code in invoked methods changes, they may 
not get inlined or EA may not be able to prove that the cloned 
instance can't escape and allocation may be introduced. 
ZoneId.systemDefault() is a hot method and it would be nice if we 
manage to keep it allocation free.


Well, I tried the following variant of SimpleTimeZone.clone() patch:

    public Object clone()
    {
    SimpleTimeZone tz = (SimpleTimeZone) super.clone();
    // like tz.invalidateCache() but without holding a lock on 
clone

    tz.cacheYear = tz.startYear - 1;
    tz.cacheStart = tz.cacheEnd = 0;
    return tz;
    }


...and the JMH benchmark with gc profiling shows that 
ZoneId.systemDefault() still manages to get JIT-compiled without 
introducing allocation.


Even the following (original Venkat's) patch:

    public Object clone()
    {
    SimpleTimeZone tz = (SimpleTimeZone) super.clone();
    tz.invalidateCache();
    return tz;
    }

...does the same and the locking in invalidateCache() is elided too. 
Allocation and lock-elision go hand-in-hand. When object doesn't 
escape, allocation on heap may be eliminated and locks on that 
instance elided.


So this is better than synchronizing on the original instance during 
.clone() execution as it has potential to avoid locking overhead.


So Venkat, go ahead. My fear was unjustified.

Regards, Peter








--
Regards
-Venkat

Re: [10] RFR 8176841: Additional Unicode Language-Tag Extensions

[Stephen Colebourne]

On 21 November 2017 at 01:45, Naoto Sato  wrote:
>> 2) `DecimalStyle.ofLocale(Locale)` should use "nu" but does not.
>
> Document it in the javadoc.

Javadoc looks good, but the webrev didn't contain matching code last
time I looked.

>> 3) `DateTimeFormatter.localizedBy(Locale)` should use "ca" to call
>> `withChronology`, `tz` to call `withZoneId` and `nu` to call
>> `withDecimalStyle`. This is a change to the CSR.
>
> Besides that "nu" needs to be spec'ed out, isn't calling with() an
> implementation note?

The revised text ends with "Same is true for the "nu" extension.", but
doesn't mention the case where both "tz" and "rg" are present.

>> 7) WeekBasedFieldPrinterParser should use "fw"/"rg", which it already
>> does via WeekFields.of(Locale)
>
> Not sure what this means. Where is the file located?

WeekBasedFieldPrinterParser is an inner class of DateTimeFormatterBuilder

>> 10) Consider how localizedBy(Locale) operates.
>
> IIRC, the localizedBy() is added so that withLocale() would behave as it is
> now. I think localizedBy() should also have the same effect as withLocale if
> the specified locale do not contain any calendar/timezone/numberingSystem
> extensions. Otherwise, say localizedBy(Locale.JAPAN) would be no-operation.

OK, I agree. localizedBy(locale) is the same as withLocale(locale)
unless there are "ca", "tz" or "nu", in which case the matching
element is updated.

thanks
Stephen

Re: RFR: JDK-8189611: JarFile versioned stream and real name support

[Alan Bateman]

On 21/11/2017 02:58, Xueming Shen wrote:

:

http://cr.openjdk.java.net/~sherman/8189611/webrev

jdeps' VersionHelper.java still accesses the "getRealName()" via the 
SharedSecrets.
Since jdeps is being compiled/built with the bootjdk, I'm leaving it 
untouched for

now.

I think we need to create an issue in JIRA to get jdeps updated. It 
actually has two issues, the other is the usage of 
jdk.internal.util.jar.VersionedStream. I'm surprised that there are 
issues with the boot JDK because it uses both in JDK 9 when the boot JDK 
is JDK 8. No issue with keeping this separate to the current effort.


I've skimmed through the latest webrev and it looks good good. The 
JarEntry::getRealName and JarFile::versionedStream APIs look good.


The update to jlink to use the standard API should mean that java.base 
no longer needs to export jdk.internal.util.jar to jdk.jlink.


JarFile is updated to import LinkedHashMap, I don't think that is needed 
in this version.


I think that is all I have for now.

-Alan

Re: RFR 8191516: OutputStream.write(byte[], int, int) could have fewer parameter bounds checks

[Brian Burkhalter]

On Nov 21, 2017, at 9:16 AM, Paul Sandoz  wrote:

>> Both good points. Updated accordingly: 
>> http://cr.openjdk.java.net/~bpb/8191516/webrev.01/.
>> 
> 
> Looks good. How about updating the read method as well to use the bounds 
> check method? (the len == 0 check makes sense there)?

Probably might as well expand it a little to see where else similar changes 
might be made.

Thanks,

Brian

Re: RFR: JDK-8189611: JarFile versioned stream and real name support

[Paul Sandoz]

> On 20 Nov 2017, at 18:58, Xueming Shen  wrote:
> 
> Thanks Paul!
> 
> Webrev has been updated as suggested.
> 
> http://cr.openjdk.java.net/~sherman/8189611/webrev
> 
> jdeps' VersionHelper.java still accesses the "getRealName()" via the 
> SharedSecrets.
> Since jdeps is being compiled/built with the bootjdk, I'm leaving it 
> untouched for
> now.
> 
> --
> 610 // placeholder for now
> 611 String getRealName(JarEntry entry) {
> 612 return entry.getRealName();
> 613 } No longer required?
> 
> I dunno what calls it...
> —
> 

Ok, I suggest adding a comment as to why it is still needed. No need for 
another review round.

Thanks,
Paul.

Re: RFR 8191516: OutputStream.write(byte[], int, int) could have fewer parameter bounds checks

[Brian Burkhalter]

On Nov 20, 2017, at 6:32 PM, Paul Sandoz  wrote:

> See also Objects.checkFromIndexSize if you wanna use that instead.
> 
> Also the if len == 0 check is probably redundant, i doubt it makes any 
> difference given the condition needs to be checked before entering the loop.

Both good points. Updated accordingly: 
http://cr.openjdk.java.net/~bpb/8191516/webrev.01/.

Thanks,

Brian

Re: [10] RFR 8176841: Additional Unicode Language-Tag Extensions

[Naoto Sato]

Thanks, Stephen.

On 11/21/17 1:35 AM, Stephen Colebourne wrote:

On 21 November 2017 at 01:45, Naoto Sato  wrote:

2) `DecimalStyle.ofLocale(Locale)` should use "nu" but does not.


Document it in the javadoc.


Javadoc looks good, but the webrev didn't contain matching code last
time I looked.


I haven't updated the webrev yet, since it will involve new test cases. 
I wanted to make sure the direction of the change was correct. Will 
update the webrev soon.





3) `DateTimeFormatter.localizedBy(Locale)` should use "ca" to call
`withChronology`, `tz` to call `withZoneId` and `nu` to call
`withDecimalStyle`. This is a change to the CSR.


Besides that "nu" needs to be spec'ed out, isn't calling with() an
implementation note?


The revised text ends with "Same is true for the "nu" extension.", but
doesn't mention the case where both "tz" and "rg" are present.


Since there is no ZoneId.ofLocale(rg), a region does not designate a 
time zone (yet). So there would be no conflict between "tz" and "rg" 
extensions.





7) WeekBasedFieldPrinterParser should use "fw"/"rg", which it already
does via WeekFields.of(Locale)


Not sure what this means. Where is the file located?


WeekBasedFieldPrinterParser is an inner class of DateTimeFormatterBuilder


OK, thanks.




10) Consider how localizedBy(Locale) operates.


IIRC, the localizedBy() is added so that withLocale() would behave as it is
now. I think localizedBy() should also have the same effect as withLocale if
the specified locale do not contain any calendar/timezone/numberingSystem
extensions. Otherwise, say localizedBy(Locale.JAPAN) would be no-operation.


OK, I agree. localizedBy(locale) is the same as withLocale(locale)
unless there are "ca", "tz" or "nu", in which case the matching
element is updated.


Good.

Naoto

Re: RFR 8191516: OutputStream.write(byte[], int, int) could have fewer parameter bounds checks

[Paul Sandoz]

> On 21 Nov 2017, at 08:47, Brian Burkhalter  
> wrote:
> 
> 
> On Nov 20, 2017, at 6:32 PM, Paul Sandoz  > wrote:
> 
>> See also Objects.checkFromIndexSize if you wanna use that instead.
>> 
>> Also the if len == 0 check is probably redundant, i doubt it makes any 
>> difference given the condition needs to be checked before entering the loop.
> 
> Both good points. Updated accordingly: 
> http://cr.openjdk.java.net/~bpb/8191516/webrev.01/ 
> .
> 

Looks good. How about updating the read method as well to use the bounds check 
method? (the len == 0 check makes sense there)?

Paul.

Re: RFR 8191516: OutputStream.write(byte[], int, int) could have fewer parameter bounds checks

[Brian Burkhalter]

On Nov 21, 2017, at 9:21 AM, Brian Burkhalter  
wrote:

>> Looks good. How about updating the read method as well to use the bounds 
>> check method? (the len == 0 check makes sense there)?
> 
> Probably might as well expand it a little to see where else similar changes 
> might be made.

Expanded to InputStream: http://cr.openjdk.java.net/~bpb/8191516/webrev.02/. 
The test java/io/InputStream/ReadParams verifies the changes to IS.

Thanks,

Brian

Re: RFR 8191516: OutputStream.write(byte[], int, int) could have fewer parameter bounds checks

[Paul Sandoz]

> On 21 Nov 2017, at 12:30, Brian Burkhalter  
> wrote:
> 
> On Nov 21, 2017, at 9:21 AM, Brian Burkhalter  > wrote:
> 
>>> Looks good. How about updating the read method as well to use the bounds 
>>> check method? (the len == 0 check makes sense there)?
>> 
>> Probably might as well expand it a little to see where else similar changes 
>> might be made.
> 
> Expanded to InputStream: http://cr.openjdk.java.net/~bpb/8191516/webrev.02/ 
> . The test 
> java/io/InputStream/ReadParams verifies the changes to IS.
> 

+1

Paul.

Re: RFR 8191173 : (cl) Clarify or remove "for delegation" in ClassLoader spec

[Martin Buchholz]

Looks good!

On Tue, Nov 21, 2017 at 12:41 PM, Brent Christian <
brent.christ...@oracle.com> wrote:

> Hi,
>
> Please review my change to this small bit of ClassLoader spec that can be
> tidied up, as noticed by Martin.
>
> Issue:
> https://bugs.openjdk.java.net/browse/JDK-8191173
> Webrev:
> http://cr.openjdk.java.net/~bchristi/8191173/webrev.00/
>
> In java.lang.ClassLoader these methods:
>   getParent()
>   getPlatformClassLoader()
>   getSystemClassLoader()
> all state that the returned classloader is, "for delegation."
>
> For getParent() this makes sense to mention.  But it seems unnecessary for
> the other two methods, which are static, and designed to always return the
> indicated classloader.  The getSystemClassLoader() docs go on to
> immediately mention that the system classloader is the default delegation
> parent.
>
> Omitting the phrase makes the spec more concise.
>
> Thanks,
> -Brent
>

Re: RFR 8191173 : (cl) Clarify or remove "for delegation" in ClassLoader spec

[David Holmes]

Seems fine.

Thanks,
David

On 22/11/2017 6:41 AM, Brent Christian wrote:

Hi,

Please review my change to this small bit of ClassLoader spec that can 
be tidied up, as noticed by Martin.


Issue:
https://bugs.openjdk.java.net/browse/JDK-8191173
Webrev:
http://cr.openjdk.java.net/~bchristi/8191173/webrev.00/

In java.lang.ClassLoader these methods:
   getParent()
   getPlatformClassLoader()
   getSystemClassLoader()
all state that the returned classloader is, "for delegation."

For getParent() this makes sense to mention.  But it seems unnecessary 
for the other two methods, which are static, and designed to always 
return the indicated classloader.  The getSystemClassLoader() docs go on 
to immediately mention that the system classloader is the default 
delegation parent.


Omitting the phrase makes the spec more concise.

Thanks,
-Brent

JDK-8191706: Proposal to add Reader::transferTo(Writer)

[Patrick Reinhart]

Hi there out a the review [1] for  JDK-8066870, that will need some more
working on, I created this specific issue to add an transferTo method to
the java.io.Reader as it seems more clear as of the similarity to
java.io.InputStream.transferTo(java.io.OutputStream) in terms of the API.

To start the discussion I got the proposed API:

/**
 * Reads all characters from this reader and writes the characters to the
 * given writer in the order that they are read. On return, this reader
 * will be at end of the data. This method does not close either reader
 * or writer.
 * 
 * This method may block indefinitely reading from the reader, or
 * writing to the writer. The behavior for the case where the reader
 * and/or writer is asynchronously closed, or the thread
 * interrupted during the transfer, is highly reader and writer
 * specific, and therefore not specified.
 * 
 * If an I/O error occurs reading from the reader or writing to the
 * writer, then it may do so after some characters have been read or
 * written. Consequently the reader may not be at end of the data and
 * one, or both, streams may be in an inconsistent state. It is strongly
 * recommended that both streams be promptly closed if an I/O error occurs.
 *
 * @param  out the writer, non-null
 * @return the number of characters transferred
 * @throws IOException if an I/O error occurs when reading or writing
 * @throws NullPointerException if {@code out} is {@code null}
 *
 * @since 10
 */
public long transferTo(Writer out) throws IOException {

}


-Patrick


[1]
http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-November/050064.html

RFR 8191173 : (cl) Clarify or remove "for delegation" in ClassLoader spec

[Brent Christian]

Hi,

Please review my change to this small bit of ClassLoader spec that can 
be tidied up, as noticed by Martin.


Issue:
https://bugs.openjdk.java.net/browse/JDK-8191173
Webrev:
http://cr.openjdk.java.net/~bchristi/8191173/webrev.00/

In java.lang.ClassLoader these methods:
  getParent()
  getPlatformClassLoader()
  getSystemClassLoader()
all state that the returned classloader is, "for delegation."

For getParent() this makes sense to mention.  But it seems unnecessary 
for the other two methods, which are static, and designed to always 
return the indicated classloader.  The getSystemClassLoader() docs go on 
to immediately mention that the system classloader is the default 
delegation parent.


Omitting the phrase makes the spec more concise.

Thanks,
-Brent

Re: JDK-8191706: Proposal to add Reader::transferTo(Writer)

[Brian Burkhalter]

Hi Patrick,

This looks fine from my point of view. It exactly mirrors 
InputStream.transferTo(OutputStream) with which I expect everyone will concur.

Thanks,

Brian

On Nov 21, 2017, at 11:56 AM, Patrick Reinhart  wrote:

> Hi there out a the review [1] for  JDK-8066870, that will need some more
> working on, I created this specific issue to add an transferTo method to
> the java.io.Reader as it seems more clear as of the similarity to
> java.io.InputStream.transferTo(java.io.OutputStream) in terms of the API.
> 
> To start the discussion I got the proposed API:
> 
> /**
>  * Reads all characters from this reader and writes the characters to the
>  * given writer in the order that they are read. On return, this reader
>  * will be at end of the data. This method does not close either reader
>  * or writer.
>  * 
>  * This method may block indefinitely reading from the reader, or
>  * writing to the writer. The behavior for the case where the reader
>  * and/or writer is asynchronously closed, or the thread
>  * interrupted during the transfer, is highly reader and writer
>  * specific, and therefore not specified.
>  * 
>  * If an I/O error occurs reading from the reader or writing to the
>  * writer, then it may do so after some characters have been read or
>  * written. Consequently the reader may not be at end of the data and
>  * one, or both, streams may be in an inconsistent state. It is strongly
>  * recommended that both streams be promptly closed if an I/O error occurs.
>  *
>  * @param  out the writer, non-null
>  * @return the number of characters transferred
>  * @throws IOException if an I/O error occurs when reading or writing
>  * @throws NullPointerException if {@code out} is {@code null}
>  *
>  * @since 10
>  */
> public long transferTo(Writer out) throws IOException {
> 
> }
> 
> 
> -Patrick
> 
> 
> [1]
> http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-November/050064.html
> 
> 
> 
> 

Re: RFR: JDK-8191706: Add Reader::transferTo(Writer)

[Brian Burkhalter]

Looks good.

Of course a corresponding CSR will need to be filed and eventually approved.

Thanks,

Brian

On Nov 21, 2017, at 2:33 PM, Patrick Reinhart  wrote:

> Here's the implementation and test..
> 
> http://cr.openjdk.java.net/~reinhapa/reviews/8191706/webrev.00
> 
> -Patrick
> 
> Am 21.11.2017 um 23:07 schrieb Brian Goetz:
>> Looks good.

Re: RFR(M) 8189116: Give the jdk.internal.vm.compiler.management only the permissions it really needs to expose the bean

[Vladimir Kozlov]

Do we have consensus about changes? I can sponsor and push it.

These changes passed Hotspot pre-integration testing and additional 
requested tests.


Thanks,
Vladimir

On 11/15/17 5:44 AM, Jaroslav Tulach wrote:

On úterý 14. listopadu 2017 21:34:45 CET mandy chung wrote:

I am wondering this ACE comes from Graal accessing jdk.vm.ci.services
from JVMCI which is defined to the boot loader versus Graal is defined
to the platform class loader.

   java.security.AccessControlException: access denied
("java.lang.RuntimePermission" "accessClassInPackage.jdk.vm.ci.services")

However Graal grants with AllPermissions.

In any case, if this is an existing issue, I am okay if you file a
separate issue to track this.


Hello Mandy,
yes, it looks like issue unrelated to my changes. I have reported: https://
bugs.openjdk.java.net/browse/JDK-8191320

I hope I haven't messed up something on my disk and others will be able to
reproduce my problem.
-jt




Mandy

On 11/14/17 2:02 PM, Sean Mullan wrote:

Try running with -Djava.security.debug=access:domain:failure

This will tell you what ProtectionDomain caused the
AccessControlException, which should give you a better idea of where
the problem is. Look for messages that start with "domain that failed ".

--Sean

On 11/14/17 1:22 AM, Jaroslav Tulach wrote:

I tried the same test with

changeset:   47679:d85284ccd1bd
user:sspitsyn
date:Fri Nov 03 17:09:25 2017 -0700
summary: 8189731: Disable CFLH when there are no transformers

and it also yields the exception. E.g. the problem is certainly not
result of
my changes.

-jt

PS: I try full rebuild on d85284ccd1bd maybe it disappears...

On pondělí 13. listopadu 2017 20:53:35 CET Jaroslav Tulach wrote:

Hello Mandy,

this was a good test:

./build/linux-x64/jdk/bin/java -XX:+UnlockExperimentalVMOptions -XX:
+EnableJVMCI -XX:+UseJVMCICompiler -jar ...


You can also try running the above command with
-Djava.security.manager
as a sanity test (the application may need additional permissions) -
just a sanity test.


I've just tried:

$ ./build/linux-x64/jdk/bin/java -XX:+UnlockExperimentalVMOptions -XX:
+EnableJVMCI -XX:+UseJVMCICompiler -Djava.security.manager -jar ~/
NetBeansProjects/sieve/java/algorithm/target/sieve-algorithm-1.0-SNAPSHO
T.ja

r

and it doesn't work. I am getting an error below, however the code
is not
running through my module at all. I don't understand the failure, I
will
have to investigate more.

-jt


Caused by: java.security.AccessControlException: access denied
("java.lang.RuntimePermission"
"accessClassInPackage.jdk.vm.ci.services")
  at java.base/
java.security.AccessControlContext.checkPermission(AccessControlContext.
java>>>
: 472)

  at java.base/
java.security.AccessController.checkPermission(AccessController.java:895
)

  at java.base/
java.lang.SecurityManager.checkPermission(SecurityManager.java:558)
  at java.base/
java.lang.SecurityManager.checkPackageAccess(SecurityManager.java:1534)
  at java.base/java.lang.ClassLoader$1.run(ClassLoader.java:680)
  at java.base/java.lang.ClassLoader$1.run(ClassLoader.java:678)
  at
java.base/java.security.AccessController.doPrivileged(Native
Method)
  at java.base/
java.lang.ClassLoader.checkPackageAccess(ClassLoader.java:678)
  at java.base/java.lang.ClassLoader.defineClass1(Native Method)
  at
java.base/java.lang.ClassLoader.defineClass(ClassLoader.java:1006) at
java.base/java.lang.ClassLoader.defineClass(ClassLoader.java:1085) at
java.base/
java.security.SecureClassLoader.defineClass(SecureClassLoader.java:206)
  at java.base/
jdk.internal.loader.BuiltinClassLoader.defineClass(BuiltinClassLoader.ja
va:7

60) at java.base/jdk.internal.loader.BuiltinClassLoader.lambda
$findClassInModuleOrNull$2(BuiltinClassLoader.java:683)
  at
java.base/java.security.AccessController.doPrivileged(Native
Method)
  at java.base/
jdk.internal.loader.BuiltinClassLoader.findClassInModuleOrNull(BuiltinCl
assL

oader.java: 684)
  at java.base/
jdk.internal.loader.BuiltinClassLoader.findClass(BuiltinClassLoader.java
:562

) at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:607) at
java.base/java.lang.Class.forName(Class.java:451)
  at java.base/java.util.ServiceLoader.lambda$loadProvider
$1(ServiceLoader.java:856)
  at
java.base/java.security.AccessController.doPrivileged(Native
Method)
  at
java.base/java.util.ServiceLoader.loadProvider(ServiceLoader.java: 858)

Re: RFR(m): 8177290 add copy factory methods for unmodifiable List, Set, Map

[John Rose]

On Nov 18, 2017, at 7:34 PM, John Rose  wrote:
> 
> On Oct 30, 2017, at 6:50 PM, Stuart Marks  wrote:
>> 
>> (also includes 8184690: add Collectors for collecting into unmodifiable 
>> List, Set, and Map)
> 
> Now I'm going to be picky about the names of the Collectors;
> please bear with me a moment.  Consider `toUnmodifiableList`
> and its two cousins (`toUSet`, `toUMap`).
> 
> The most natural names `toList` (etc.) are already taken.
> They mean "do whatever is most useful for the collector,
> perhaps returning unmodifiable or modifiable results".

Let me adjust my position here, FTR.  I am now aware (thanks
Brian) that `toList` is not broken but intentionally under-specified,
pending future changes.  It is my personal hope that the future
changes will specify that the result of `toList` is safely publishable
and an unmodifiable non-view.  This issue is tracked as JDK-8180352.

(Logically possible alternatives would seem to include an otherwise
unconstrained mutable list, an ArrayList, or a continuation of the
"Chef's Choice" policy in effect today.  I suspect we could find
advocates for all of those positions, as evidenced by comments
on JDK-8180352.  I just added mine for the record.)

If at some point in the future `toList` does produce the same
kind of safe list as `List.of`, then I won't have anything to be
picky about.  Other folks can use `toCollection(ArrayList::new)`
or some other explicit op-in for mutability.

For now, a security-conscious user like me can work with
`Collections.toUnmodifiableList`.  That API point will (I hope)
have a short career, ending when `toList` does something at
least as useful.

If at some point in the future `toList` switches to guarantee
the mutable ArrayList (as it seems to supply today), then the
hope for a simple and safe `toList` will be dashed, and we
will have to look for something else that is explicitly oriented
towards value based classes, such as `Collectors.valueList`
and/or `Stream.values`.

Value-based classes are an important "attractor" for API
design, because they can be simultaneously safe and
performant, compared to Java arrays and ArrayList.

(The performance comes from the elimination of copies
under the hood, as well as structure flattening, optimizations
which potential modification makes impossible.  The safety
comes from reduction of difficult-to-find TOCTTOU attack
surfaces, as beyond the usual claimed benefits of "FP style".)

When we get to Valhalla value types, there will be many
more value-based classes in the world, since a value
type cannot be anything other than a value-based class.
Whatever we do with `toList` in the future, it should take
into account value-based usages.  They are here and
more are coming.

One more point:  When programming with value-based
classes, the container is almost irrelevant, and the contents
are the whole story.  Thus, API points which return value-based
multiple values should probably not mention the container
type (List) unless there is a real danger of ambiguity.
If I have a tree node type and I want to use a value-based
List to encode its children, it should look like this:

   List children() { return this.children; }

or this:

   List children() { return List.copyOf(this.children); }

not these:

   List childrenList() ...
   List listOfChildren() ...
   List childrenUnmodifiableList() ...
   private List secretChildrenArray() ...

(etc.)

An API in the value-based style could say, in a class header or
package intro page, that collection values are value-based if not
otherwise specified, and then simply use unmodifiable building
blocks everywhere.

In any case, I think the de-emphasis of container identity enabled
by value-based types provides a subtle but strong push on API
design, towards plural nouns, and away from explicit discussion
of container details.

— John

P.S. Ten years ago I designed the MethodType API, which talks 
about a single return type and multiple parameter types.  The
corresponding API points are returnType and (not parameterTypes
but) parameterList *and* parameterArray.  Because at the time
arrays and lists were both heavily used carriers for multiple values,
we had to focus on the box types (List, Array).  Also we didn't
have VBC rules yet.  In the future I hope similar API designs can
(a) ignore legacy arrays, and (b) just use VBC lists, and then
(c) be a little less noisy about how the data is carried around.

P.P.S. Fully retiring arrays will require dislodging them from
their favored position with varargs and similar spots in the stack.

Re: RFR(m): 8177290 add copy factory methods for unmodifiable List, Set, Map

[John Rose]

On Nov 18, 2017, at 8:10 AM, Brian Goetz  wrote:
> 
> I'm with Remi on this.  
> 
> Sent from my MacBook Wheel
> 
>> On Nov 18, 2017, at 5:41 AM, Remi Forax > > wrote:
>> 
>> Hi John,
>> i strongly believe that static fluent methods have no place in a blue collar 
>> language
>> ...

>> So in my opinion, the only possible option is to introduce final default 
>> methods, i fully understand that this is non trivial to introduce this kind 
>> of methods in the VM but this discussion is a good use case for their 
>> introduction. 

We have four choices on the table with respect to the occasional
need for securable API points in fluent APIs:

0. Blue collar language:  Pick only one of fluency or security.

1. Secure a default method by marking it final.

2. Secure a fluent API point by binding to a static in the same type.

4. Extension methods:  Anybody can "import" new statics into any type.

I think #0 hurts security, which is why I keep objecting to it.

Brian thinks #1 puts too many restrictions on implementors.

Although it would seem to allow everybody to do whatever they want,
#4 is not a fit for Java.  APIs in java.base are carefully curated,
and allowing third parties to add "nice" methods would interfere
with that curation.

Option #2 has the known good properties of C# extension methods
(decoupling from receiver dispatch, natural notation), without the known
bad properties of C# extension methods (lack of curation).

So #2 is the least ugly solution for an ugly problem, except possibly #1
Which I would accept also.  I would also be glad to see a #5 that would
please everybody.

— John

P.S. Security is a big concern for us developers of java.base.  And it is
surely a concern for everyone else, at least in part.  If you program
behind only a firewall, consider that program integrity and robustness
are corollaries of security.  Your past self and teammates are throwing
buggy code at your present self; you want to be secure from that
even if you don't fear nefarious attackers.  When we secure the
Java stack from nefarious attackers we are also preventing large
numbers of unintentional bugs.

Re: JDK-8191706: Proposal to add Reader::transferTo(Writer)

[Brian Goetz]

Looks good.

On 11/21/2017 2:56 PM, Patrick Reinhart wrote:

Hi there out a the review [1] for  JDK-8066870, that will need some more
working on, I created this specific issue to add an transferTo method to
the java.io.Reader as it seems more clear as of the similarity to
java.io.InputStream.transferTo(java.io.OutputStream) in terms of the API.

To start the discussion I got the proposed API:

/**
  * Reads all characters from this reader and writes the characters to the
  * given writer in the order that they are read. On return, this reader
  * will be at end of the data. This method does not close either reader
  * or writer.
  * 
  * This method may block indefinitely reading from the reader, or
  * writing to the writer. The behavior for the case where the reader
  * and/or writer is asynchronously closed, or the thread
  * interrupted during the transfer, is highly reader and writer
  * specific, and therefore not specified.
  * 
  * If an I/O error occurs reading from the reader or writing to the
  * writer, then it may do so after some characters have been read or
  * written. Consequently the reader may not be at end of the data and
  * one, or both, streams may be in an inconsistent state. It is strongly
  * recommended that both streams be promptly closed if an I/O error occurs.
  *
  * @param  out the writer, non-null
  * @return the number of characters transferred
  * @throws IOException if an I/O error occurs when reading or writing
  * @throws NullPointerException if {@code out} is {@code null}
  *
  * @since 10
  */
public long transferTo(Writer out) throws IOException {

}


-Patrick


[1]
http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-November/050064.html

RFR 8191736: replace javah w/ javac in jdk tests

[Alexandre (Shura) Iline]

Hi.

Please take look on this quick fix of removing remains of javah usages from 
test/jdk.

Bug: https://bugs.openjdk.java.net/browse/JDK-8191736
Webrev: http://cr.openjdk.java.net/~shurailine/8191736/webrev.00/

Thank you.

Shura

RFR: JDK-8191706: Add Reader::transferTo(Writer)

[Patrick Reinhart]

Here's the implementation and test..

http://cr.openjdk.java.net/~reinhapa/reviews/8191706/webrev.00

-Patrick

Am 21.11.2017 um 23:07 schrieb Brian Goetz:
> Looks good.
>
> On 11/21/2017 2:56 PM, Patrick Reinhart wrote:
>> Hi there out a the review [1] for  JDK-8066870, that will need some more
>> working on, I created this specific issue to add an transferTo method to
>> the java.io.Reader as it seems more clear as of the similarity to
>> java.io.InputStream.transferTo(java.io.OutputStream) in terms of the
>> API.
>>
>> To start the discussion I got the proposed API:
>>
>> /**
>>   * Reads all characters from this reader and writes the characters
>> to the
>>   * given writer in the order that they are read. On return, this reader
>>   * will be at end of the data. This method does not close either reader
>>   * or writer.
>>   * 
>>   * This method may block indefinitely reading from the reader, or
>>   * writing to the writer. The behavior for the case where the reader
>>   * and/or writer is asynchronously closed, or the thread
>>   * interrupted during the transfer, is highly reader and writer
>>   * specific, and therefore not specified.
>>   * 
>>   * If an I/O error occurs reading from the reader or writing to the
>>   * writer, then it may do so after some characters have been read or
>>   * written. Consequently the reader may not be at end of the data and
>>   * one, or both, streams may be in an inconsistent state. It is
>> strongly
>>   * recommended that both streams be promptly closed if an I/O error
>> occurs.
>>   *
>>   * @param  out the writer, non-null
>>   * @return the number of characters transferred
>>   * @throws IOException if an I/O error occurs when reading or writing
>>   * @throws NullPointerException if {@code out} is {@code null}
>>   *
>>   * @since 10
>>   */
>> public long transferTo(Writer out) throws IOException {
>> 
>> }
>>
>>
>> -Patrick
>>
>>
>> [1]
>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-November/050064.html
>>
>>
>>
>>
>>
>

Re: RFR: JDK-8191706: Add Reader::transferTo(Writer)

[Patrick Reinhart]

Created

https://bugs.openjdk.java.net/browse/JDK-8191737

Any additions welcome

-Patrick


Am 21.11.2017 um 23:41 schrieb Brian Burkhalter:
> Looks good.
>
> Of course a corresponding CSR will need to be filed and eventually
> approved.
>
> Thanks,
>
> Brian
>
> On Nov 21, 2017, at 2:33 PM, Patrick Reinhart  > wrote:
>
>> Here's the implementation and test..
>>
>> http://cr.openjdk.java.net/~reinhapa/reviews/8191706/webrev.00
>> 
>>
>> -Patrick
>>
>> Am 21.11.2017 um 23:07 schrieb Brian Goetz:
>>> Looks good.
>

Re: [10] RFR 8191404: Upgrading JDK with latest available LSR data from IANA.

[Naoto Sato]

Hi Nishit,

One nit in the test. At line 71, probably the original intention of 
"str" is to list the accept languages in alphabetic order. Would you 
change it?


I have not looked into actual tags in the test. Did  you modify them 
manually? If so, it would be desirable to make it automated.


Naoto

On 11/16/17 3:47 AM, Nishit Jain wrote:

Hi,

Please review the fix for JDK-8191404

Bug: https://bugs.openjdk.java.net/browse/JDK-8191404
Webrev: http://cr.openjdk.java.net/~nishjain/8191404/webrev.00/

Fix: Updated the LSR data with the version: 2017-08-15

Regards,
Nishit Jain

Re: RFR 8191736: replace javah w/ javac in jdk tests

[Jonathan Gibbons]

Shura,

Looks OK to me, although I'm not normally a Reviewer for AWT tests.

-- Jon


On 11/21/17 3:58 PM, Alexandre (Shura) Iline wrote:

Hi.

Please take look on this quick fix of removing remains of javah usages from 
test/jdk.

Bug: https://bugs.openjdk.java.net/browse/JDK-8191736
Webrev: http://cr.openjdk.java.net/~shurailine/8191736/webrev.00/

Thank you.

Shura

Re: RFR 8191736: replace javah w/ javac in jdk tests

[Sergey Bylokhov]

Looks fine.

On 21/11/2017 16:09, Jonathan Gibbons wrote:

Shura,

Looks OK to me, although I'm not normally a Reviewer for AWT tests.

-- Jon


On 11/21/17 3:58 PM, Alexandre (Shura) Iline wrote:

Hi.

Please take look on this quick fix of removing remains of javah usages 
from test/jdk.


Bug: https://bugs.openjdk.java.net/browse/JDK-8191736
Webrev: http://cr.openjdk.java.net/~shurailine/8191736/webrev.00/

Thank you.

Shura






--
Best regards, Sergey.