Re: Review Request: 7193406 - Clean-up JDK Build Warnings in java.util, java.io

2012-08-29 Thread Joe Darcy
On 8/29/2012 7:07 PM, Stuart Marks wrote: On 8/29/12 4:56 PM, Joe Darcy wrote: On 8/29/2012 4:20 PM, Stuart Marks wrote: The original code was like this: 427 private static int getMask(String actions) { ... 435 // Check against use of constants (used heavily within

Re: Review Request: 7193406 - Clean-up JDK Build Warnings in java.util, java.io

2012-08-29 Thread David Holmes
Hi Stuart, On 30/08/2012 8:53 AM, Stuart Marks wrote: On 8/29/12 8:48 AM, Andrew Hughes wrote: But presumably [-Werror] would be removed when everything is warning free? -Werror should not be the default for everyone building OpenJDK, who then end up having to fix or workaround issues which are

Re: Review Request: 7193406 - Clean-up JDK Build Warnings in java.util, java.io

2012-08-29 Thread Stuart Marks
On 8/29/12 4:56 PM, Joe Darcy wrote: On 8/29/2012 4:20 PM, Stuart Marks wrote: The original code was like this: 427 private static int getMask(String actions) { ... 435 // Check against use of constants (used heavily within the JDK) 436 if (actions == Secur

RFR: 7195099 update problem list with RMI test failures

2012-08-29 Thread Stuart Marks
Hi Alan, I'm opening another front in the war against test failures. Please review these additions to the problem list. Thanks. s'marks diff -r c4c69b4d9ace test/ProblemList.txt --- a/test/ProblemList.txt Wed Aug 29 11:03:02 2012 +0800 +++ b/test/ProblemList.txt Wed Aug 29 19:00:5

Re: Review Request: 7193406 - Clean-up JDK Build Warnings in java.util, java.io

2012-08-29 Thread Joe Darcy
On 8/29/2012 4:20 PM, Stuart Marks wrote: On 8/29/12 4:36 AM, Ulf Zibis wrote: 436 switch (actions) { 437 case SecurityConstants.FILE_READ_ACTION: 438 return READ; Oops, you have reverted the change to use switch-on-Strings by webrev.03. Why? A fair qu

Re: Review Request: 7193406 - Clean-up JDK Build Warnings in java.util, java.io

2012-08-29 Thread Dan Xu
On 08/29/2012 12:11 PM, Dan Xu wrote: On 08/29/2012 08:27 AM, Joe Darcy wrote: Hello, On 8/29/2012 1:48 AM, Rémi Forax wrote: On 08/29/2012 08:33 AM, Kurchi Subhra Hazra wrote: Thanks for cleaning up those spaces Dan. The changes look fine. Sorry for the extra trouble! - Kurchi On 8/28/12

Re: RFR [JDK8]: 7169894: JAXP Plugability Layer: using service loader

2012-08-29 Thread Joe Wang
Paul, Alan, Confusion was what jaxp meant to give :) I was told that the factory/object finders, security support classes were duplicated, and needed to be kept in sync. But they are not even in their original form, unfortunately. Both of you mentioned that it's desirable to make SCE the

Re: Review Request: 7193406 - Clean-up JDK Build Warnings in java.util, java.io

2012-08-29 Thread Stuart Marks
On 8/29/12 4:36 AM, Ulf Zibis wrote: 436 switch (actions) { 437 case SecurityConstants.FILE_READ_ACTION: 438 return READ; Oops, you have reverted the change to use switch-on-Strings by webrev.03. Why? A fair question. I had instigated some internal conve

Re: RFR [JDK8]: 7169894: JAXP Plugability Layer: using service loader

2012-08-29 Thread Joe Wang
I actually updated the webrev yesterday with your suggestion. Recall our discussions back in June, the suggestion was to delegate to the ServiceLoader, e.g. ServiceLoader.load(serviceClass). The rational was that the ServiceLoader uses context and then bootstrap class loader. The spec for pub

Re: Review Request: 7193406 - Clean-up JDK Build Warnings in java.util, java.io

2012-08-29 Thread Stuart Marks
On 8/29/12 3:50 AM, Ulf Zibis wrote: In FilePermission.java file, I make one change to its method signature, public Enumeration elements() ==> public Enumeration elements() Actually the whole method is synchronized. To make this more clear, I suggest: 798 public synchronized E

Re: Review Request: 7193406 - Clean-up JDK Build Warnings in java.util, java.io

2012-08-29 Thread Stuart Marks
On 8/29/12 8:48 AM, Andrew Hughes wrote: But presumably [-Werror] would be removed when everything is warning free? -Werror should not be the default for everyone building OpenJDK, who then end up having to fix or workaround issues which are nothing to do with them. It's easy enough for those w

Re: Review Request: 7193710 ByteArrayOutputStream Javadoc contains unclosed element

2012-08-29 Thread Dan Xu
Thanks for your comments. I have updated my fix and posted at, http://cr.openjdk.java.net/~dxu/7193710/webrev.01/. I think the effort to upgrade to use new tags is out of the scope of this fix and will be addressed against the whole JDK in one chunk later. -Dan On 08/28/2012 05:13 PM, Dan X

Re: Review Request: 7193406 - Clean-up JDK Build Warnings in java.util, java.io

2012-08-29 Thread Dan Xu
On 08/29/2012 08:27 AM, Joe Darcy wrote: Hello, On 8/29/2012 1:48 AM, Rémi Forax wrote: On 08/29/2012 08:33 AM, Kurchi Subhra Hazra wrote: Thanks for cleaning up those spaces Dan. The changes look fine. Sorry for the extra trouble! - Kurchi On 8/28/12 10:22 PM, Dan Xu wrote: It is funny. :)

Re: Review Request: 7193683: Cleanup Warning in java.sql package

2012-08-29 Thread Lance Andersen - Oracle
This looks fine. Dan, I will commit this for you Thursday Best Lance On Aug 29, 2012, at 1:29 AM, Dan Xu wrote: > I made a simple fix to clean up build warnings in java.sql package. The > change can be reviewed at http://cr.openjdk.java.net/~dxu/7193683/webrev.01/. > Thanks! > > -Dan Lance

Re: RFR [JDK8]: 7169894: JAXP Plugability Layer: using service loader

2012-08-29 Thread Paul Sandoz
On Aug 29, 2012, at 10:56 AM, Paul Sandoz wrote: > Hi Joe, > > I would urge you to reconsider errors using SL, since SL is being explicitly > called out as part of the specification. > > You can make any such SL-related errors more meaningful (yes, i want the > stack trace telling what bits

Re: Review Request: 7193406 - Clean-up JDK Build Warnings in java.util, java.io

2012-08-29 Thread Andrew Hughes
- Original Message - > On 8/24/12 2:42 AM, Andrew Hughes wrote: > > However, once the whole build is warning free, would it not be > > preferable > > to remove these and just set JAVAC_WARNINGS_FATAL=true when doing > > development > > builds? > > > > The problem I see is someone new comi

Re: Review Request: 7193406 - Clean-up JDK Build Warnings in java.util, java.io

2012-08-29 Thread Joe Darcy
Hello, On 8/29/2012 1:48 AM, Rémi Forax wrote: On 08/29/2012 08:33 AM, Kurchi Subhra Hazra wrote: Thanks for cleaning up those spaces Dan. The changes look fine. Sorry for the extra trouble! - Kurchi On 8/28/12 10:22 PM, Dan Xu wrote: It is funny. :) I have searched all source codes under jdk

Re: Review Request: 7193406 - Clean-up JDK Build Warnings in java.util, java.io

2012-08-29 Thread Ulf Zibis
Am 29.08.2012 07:22, schrieb Dan Xu: It is funny. :) I have searched all source codes under jdk and removed spaces for the similar cases. Please review the new version of change at, http://cr.openjdk.java.net/~dxu/7193406/webrev.03/. Thanks for your comment! In class j.u.Collections you hav

Re: Review Request: 7193406 - Clean-up JDK Build Warnings in java.util, java.io

2012-08-29 Thread Ulf Zibis
Am 24.08.2012 21:07, schrieb Dan Xu: On 08/23/2012 06:53 PM, David Holmes wrote: I'm surprised that you need this: 426 @SuppressWarnings("fallthrough") ... 436 switch (actions) { 437 case SecurityConstants.FILE_READ_ACTION: 438 return READ; Oops, y

Re: Review Request: 7193406 - Clean-up JDK Build Warnings in java.util, java.io

2012-08-29 Thread Ulf Zibis
Am 24.08.2012 02:12, schrieb Andrew Hughes: In FilePermission.java file, I make one change to its method signature, public Enumeration elements() ==> public Enumeration elements() I am not sure whether it will cause an issue of backward compatibility. Please advise. Thanks! Actuall

Re: Difference between Files#delete(Path) and File#delete()

2012-08-29 Thread Alan Bateman
On 29/08/2012 11:15, Premraj wrote: I was trying to shift from old file API to NIO API (using JDK 7 update 6) but I found that on Windows (I have Windows 7) platform if I have read only file then File#delete() call will delete the file while Files#delete(Path) fails with exception (java.nio.file

Re: RFR [JDK8]: 7169894: JAXP Plugability Layer: using service loader

2012-08-29 Thread Alan Bateman
On 28/08/2012 05:57, Joe Wang wrote: : In DocumentBuilderFactory and SAXParserFactory the javadoc reads " Otherwise the default implementation is returned if it is on the classpath or installed as a module". I think this statement needs to be re-worked, first to remove the word "module" as

Difference between Files#delete(Path) and File#delete()

2012-08-29 Thread Premraj
Hi all, I'm not sure whether this is the right place for this question but if in case its not please direct me to the correct forum. I was trying to shift from old file API to NIO API (using JDK 7 update 6) but I found that on Windows (I have Windows 7) platform if I have read only file then File

Re: Review Request: 7193683: Cleanup Warning in java.sql package

2012-08-29 Thread Ulf Zibis
Looks good! While you are there maybe correct indentations and tab -> spaces, e.g.: 531 } catch(Throwable t) { 532 // Do nothing 533 } But here maybe better: 531 } catch(Throwable t) {} // Do nothing -Ulf Am 29.08.2012 07:29,

Re: RFR [JDK8]: 7169894: JAXP Plugability Layer: using service loader

2012-08-29 Thread Paul Sandoz
Hi Joe, I would urge you to reconsider errors using SL, since SL is being explicitly called out as part of the specification. You can make any such SL-related errors more meaningful (yes, i want the stack trace telling what bits of SL code were called!) and remove any potential for CCEs, due

Re: Review Request: 7193406 - Clean-up JDK Build Warnings in java.util, java.io

2012-08-29 Thread Rémi Forax
On 08/29/2012 08:33 AM, Kurchi Subhra Hazra wrote: Thanks for cleaning up those spaces Dan. The changes look fine. Sorry for the extra trouble! - Kurchi On 8/28/12 10:22 PM, Dan Xu wrote: It is funny. :) I have searched all source codes under jdk and removed spaces for the similar cases. Ple

Re: [7u8] Request for approval: 7160252: (prefs) NodeAddedEvent was not delivered when new node add when new Node

2012-08-29 Thread Edvard Wendelin
Sorry for the delay. Approved. Cheers, Edvard Ps. Please start new threads instead of replying to an old approval request. On 08/28/2012 02:22 AM, Kurchi Hazra wrote: This is a request for approval to backport the fix for 7160252 from 8 to 7u8. Bug:http://bugs.sun.com/bugdatabase/view_bu