Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]

2011-08-16 Thread Dr Andrew John Hughes
On 17:11 Tue 02 Aug , Alan Bateman wrote: > Xuelei Fan wrote: > > : > > 1. I noticed the copyright date of a few files are unchanged, please > > update them before you push the changes. > > > This has come up a few times but I don't think it is strictly required. > Kelly or one of the relea

Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]

2011-08-09 Thread Alexandre Boulgakov
:00 US/Canada Pacific Subject: Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror] On Aug 9, 2011, at 10:16 AM, Alexandre Boulgakov wrote: I can change it back to LdapNamingEnumeration. I just thought it would be more consistent with LdapB

Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]

2011-08-09 Thread Xuelei Fan
ent: Monday, August 8, 2011 7:30:09 PM GMT -08:00 US/Canada Pacific > Subject: Re: [Fwd: Code review request: 7072353 JNDI libraries do not build > with javac -Xlint:all -Werror] > > > > On Aug 9, 2011, at 10:16 AM, Alexandre Boulgakov > wrote: > >> I can

Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]

2011-08-09 Thread Alexandre Boulgakov
Sent: Monday, August 8, 2011 7:30:09 PM GMT -08:00 US/Canada Pacific Subject: Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror] On Aug 9, 2011, at 10:16 AM, Alexandre Boulgakov wrote: > I can change it back to LdapNamingEnumeration. I just thought

Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]

2011-08-08 Thread Xuelei Fan
> To: alexandre.boulga...@oracle.com > Cc: joe.da...@oracle.com, core-libs-dev@openjdk.java.net > Sent: Monday, August 8, 2011 6:58:41 PM GMT -08:00 US/Canada Pacific > Subject: Re: [Fwd: Code review request: 7072353 JNDI libraries do not build > with javac -Xlint:all -Werror] > > On 8/

Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]

2011-08-08 Thread Alexandre Boulgakov
om To: alexandre.boulga...@oracle.com Cc: joe.da...@oracle.com, core-libs-dev@openjdk.java.net Sent: Monday, August 8, 2011 6:58:41 PM GMT -08:00 US/Canada Pacific Subject: Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror] On 8/6/2011 2:11 AM, Alex

Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]

2011-08-08 Thread Xuelei Fan
On 8/6/2011 2:11 AM, Alexandre Boulgakov wrote: > Here's the second version: > http://cr.openjdk.java.net/~mduigou/7072353/2/webrev/ > > > * Changed LdapResult.referrals to be a Vector>; > * Refactored > o com/sun/jndi/dns/DnsCont

Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]

2011-08-05 Thread Alexandre Boulgakov
Here's the second version: http://cr.openjdk.java.net/~mduigou/7072353/2/webrev/ * Changed LdapResult.referrals to be a Vector>; * Refactored o com/sun/jndi/dns/DnsContext.java: BaseNameClassPairEnumeration; o com/sun/jndi/to

Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]

2011-08-05 Thread Sebastian Sickelmann
I like that idea. Should we open another discussion thread? Colin Decker schrieb: >One better way to handle this in Java 8 would be to have a utility method >that takes a Supplier> SAM argument (with a no-arg method >that returns an Enumeration) and returns an Iterable that gets a new >Enumerat

Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]

2011-08-04 Thread Alexandre Boulgakov
On 8/3/2011 10:44 PM, Joe Darcy wrote: David Holmes wrote: Joe Darcy said the following on 08/04/11 12:33: David Holmes wrote: Using wildcards makes the subtyping work along the type argument axis. So what is the right fix here? To declare the underlying Vector as a Vector and cast it to s

Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]

2011-08-04 Thread Rémi Forax
On 08/04/2011 07:39 PM, Colin Decker wrote: Well, Iterator doesn't have an iterator() method. It also looks like you'd have to have a reference to a single Enumeration already there? I was suggesting using a method reference to an Enumeration-returning method so that a fresh Enumeration could b

Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]

2011-08-04 Thread Colin Decker
Well, Iterator doesn't have an iterator() method. It also looks like you'd have to have a reference to a single Enumeration already there? I was suggesting using a method reference to an Enumeration-returning method so that a fresh Enumeration could be retrieved to back each Iterator created by the

Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]

2011-08-04 Thread Rémi Forax
On 08/04/2011 06:52 PM, Colin Decker wrote: No, that copies the Enumeration. I'm talking about something that creates lazy Iterators backed by Enumerations. -- Colin Ok, why not adding a method iterator(Enumeration) that takes an Enumeration and returns an Iterator and then do a method refere

Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]

2011-08-04 Thread Colin Decker
No, that copies the Enumeration. I'm talking about something that creates lazy Iterators backed by Enumerations. -- Colin On Thu, Aug 4, 2011 at 12:34 PM, Rémi Forax wrote: > On 08/04/2011 06:17 PM, Colin Decker wrote: > >> One better way to handle this in Java 8 would be to have a utility me

Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]

2011-08-04 Thread Rémi Forax
On 08/04/2011 06:17 PM, Colin Decker wrote: One better way to handle this in Java 8 would be to have a utility method that takes a Supplier> SAM argument (with a no-arg method that returns an Enumeration) and returns an Iterable that gets a new Enumeration from the supplier each time iterator()

Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]

2011-08-04 Thread Colin Decker
One better way to handle this in Java 8 would be to have a utility method that takes a Supplier> SAM argument (with a no-arg method that returns an Enumeration) and returns an Iterable that gets a new Enumeration from the supplier each time iterator() is called. It could then be used with method re

Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]

2011-08-03 Thread Joe Darcy
David Holmes wrote: Joe Darcy said the following on 08/04/11 12:33: David Holmes wrote: Using wildcards makes the subtyping work along the type argument axis. So what is the right fix here? To declare the underlying Vector as a Vector and cast it to something concrete when needed? It seems

Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]

2011-08-03 Thread David Holmes
Joe Darcy said the following on 08/04/11 12:33: David Holmes wrote: Using wildcards makes the subtyping work along the type argument axis. So what is the right fix here? To declare the underlying Vector as a Vector and cast it to something concrete when needed? It seems very wrong to me to b

Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]

2011-08-03 Thread Joe Darcy
PS Looking in src/share/classes/com/sun/jndi/dns/DnsContext.java, for this code 1085 public Binding nextElement() { 1086 try { 1087 return next(); 1088 } catch (NamingException e) { 1089 throw (new java.util.NoSuchElementException( 1090

Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]

2011-08-03 Thread Joe Darcy
David Holmes wrote: Joe Darcy said the following on 08/04/11 02:24: On 8/3/2011 12:42 AM, David Holmes wrote: Alexandre Boulgakov said the following on 08/03/11 04:44: On 8/2/2011 2:19 AM, Xuelei Fan wrote: 3017 Vector temp = (Vector)extractURLs(res.errorMessage); You may not need the co

Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]

2011-08-03 Thread David Holmes
Joe Darcy said the following on 08/04/11 02:24: On 8/3/2011 12:42 AM, David Holmes wrote: Alexandre Boulgakov said the following on 08/03/11 04:44: On 8/2/2011 2:19 AM, Xuelei Fan wrote: 3017 Vector temp = (Vector)extractURLs(res.errorMessage); You may not need the conversion any more, th

Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]

2011-08-03 Thread Xuelei Fan
On 8/4/2011 2:03 AM, Alexandre Boulgakov wrote: > Please see my responses inline. > > Thanks! > -Sasha > > On 8/2/2011 9:13 PM, Xuelei Fan wrote: >> . com/sun/jndi/toolkit/dir/SearchFilter.java >> 451 for (NamingEnumeration ve = attr.getAll(); >> 452 ve.hasMore(); >> 4

Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]

2011-08-03 Thread Alexandre Boulgakov
Users of Iterable expect to call Iterable.iterator() multiple times, receiving a fresh iterator pointing to the beginning of the Iterable each time. This would not be possible to do with an Enumeration wrapper since Enumerations are read-once. I don't know if this is a strong enough reason not

Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]

2011-08-03 Thread Neil Richards
On Wed, 2011-08-03 at 11:03 -0700, Alexandre Boulgakov wrote: > Please see my responses inline. > > Thanks! > -Sasha > > On 8/2/2011 9:13 PM, Xuelei Fan wrote: > > . com/sun/jndi/toolkit/dir/SearchFilter.java > > 451 for (NamingEnumeration ve = attr.getAll(); > > 452 ve.

Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]

2011-08-03 Thread Alexandre Boulgakov
On 8/3/2011 10:51 AM, Dr Andrew John Hughes wrote: On 09:24 Wed 03 Aug , Joe Darcy wrote: On 8/3/2011 12:42 AM, David Holmes wrote: Alexandre Boulgakov said the following on 08/03/11 04:44: On 8/2/2011 2:19 AM, Xuelei Fan wrote: 3017 Vector temp = (Vector)extractURLs(res.errorMessage);

Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]

2011-08-03 Thread Alexandre Boulgakov
Please see my responses inline. Thanks! -Sasha On 8/2/2011 9:13 PM, Xuelei Fan wrote: . com/sun/jndi/toolkit/dir/SearchFilter.java 451 for (NamingEnumeration ve = attr.getAll(); 452 ve.hasMore(); 453) { The update is OK. But the coding style looks un

Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]

2011-08-03 Thread Dr Andrew John Hughes
On 09:24 Wed 03 Aug , Joe Darcy wrote: > On 8/3/2011 12:42 AM, David Holmes wrote: > > Alexandre Boulgakov said the following on 08/03/11 04:44: > >> On 8/2/2011 2:19 AM, Xuelei Fan wrote: > >>> 3017 Vector temp = (Vector)extractURLs(res.errorMessage); > >>> You may not need the conversion

Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]

2011-08-03 Thread Joe Darcy
On 8/3/2011 12:42 AM, David Holmes wrote: Alexandre Boulgakov said the following on 08/03/11 04:44: On 8/2/2011 2:19 AM, Xuelei Fan wrote: 3017 Vector temp = (Vector)extractURLs(res.errorMessage); You may not need the conversion any more, the return value of extractURLs() has been updated

Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]

2011-08-03 Thread David Holmes
Alexandre Boulgakov said the following on 08/03/11 04:44: On 8/2/2011 2:19 AM, Xuelei Fan wrote: 3017 Vector temp = (Vector)extractURLs(res.errorMessage); You may not need the conversion any more, the return value of extractURLs() has been updated to 2564 private static Vector extr

Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]

2011-08-02 Thread Xuelei Fan
com/sun/jndi/toolkit/dir/SearchFilter.java 451 for (NamingEnumeration ve = attr.getAll(); 452 ve.hasMore(); 453) { The update is OK. But the coding style looks uncomfortable. Would you mind change it to use for-each style? . javax/naming/directory/BasicAtt

Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]

2011-08-02 Thread Alexandre Boulgakov
Thanks for reviewing! See my responses inline. I'll wait on sending another webrev until I've received the rest of your comments. -Sasha On 8/2/2011 2:19 AM, Xuelei Fan wrote: Please review these JNDI changes. Bug detail: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7072353 webrev: htt

Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]

2011-08-02 Thread Alexandre Boulgakov
Thanks for reviewing! Please see my responses inline. I'll wait on sending another webrev until I've received the rest of your comments. -Sasha On 8/2/2011 2:19 AM, Xuelei Fan wrote: Please review these JNDI changes. Bug detail: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7072353 webr

Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]

2011-08-02 Thread Xuelei Fan
On Aug 3, 2011, at 12:11 AM, Alan Bateman wrote: > Xuelei Fan wrote: >> : >> 1. I noticed the copyright date of a few files are unchanged, please >> update them before you push the changes. >> > This has come up a few times but I don't think it is strictly required. Kelly > or one of the rel

Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]

2011-08-02 Thread Alan Bateman
Xuelei Fan wrote: : 1. I noticed the copyright date of a few files are unchanged, please update them before you push the changes. This has come up a few times but I don't think it is strictly required. Kelly or one of the release engineers run a script over the forest periodically to fix up

Re: [Fwd: Code review request: 7072353 JNDI libraries do not build with javac -Xlint:all -Werror]

2011-08-02 Thread Xuelei Fan
> Please review these JNDI changes. > Bug detail: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7072353 > webrev: http://cr.openjdk.java.net/~mduigou/7072353/0/webrev/ Thanks for your effort to make JNDI free of compile-warning. The work is hard, I appreciate it. 1. I noticed the copyright d