Re: 8000354: (props) Properties.storeToXML/loadFromXML need to allow for alternative implementations

2012-10-06 Thread Alan Bateman
On 05/10/2012 22:31, Remi Forax wrote: Hi Alan, just some minor nits, in Properties.XmlSupport, 'provider' should be PROVIDER because it's a constant and instead of using loadProvider(), you should initialize PROVIDER in a static block. line 1172, you declare a provider and line 1174, you

Re: 8000354: (props) Properties.storeToXML/loadFromXML need to allow for alternative implementations

2012-10-06 Thread Remi Forax
On 10/06/2012 01:30 PM, Alan Bateman wrote: On 05/10/2012 22:31, Remi Forax wrote: Hi Alan, just some minor nits, in Properties.XmlSupport, 'provider' should be PROVIDER because it's a constant and instead of using loadProvider(), you should initialize PROVIDER in a static block. line 1172,

Re: 8000354: (props) Properties.storeToXML/loadFromXML need to allow for alternative implementations

2012-10-06 Thread Alan Bateman
On 06/10/2012 13:42, Remi Forax wrote: Thanks Alan, I'm fine with methods loadProviderAsService and loadProviderFromProperty, but I think you can remove loadProvider because PROVIDER can be initialized in a static block. private static final XmlPropertiesProvider PROVIDER; static {

Re: 8000354: (props) Properties.storeToXML/loadFromXML need to allow for alternative implementations

2012-10-06 Thread Remi Forax
On 10/06/2012 03:05 PM, Alan Bateman wrote: On 06/10/2012 13:42, Remi Forax wrote: Thanks Alan, I'm fine with methods loadProviderAsService and loadProviderFromProperty, but I think you can remove loadProvider because PROVIDER can be initialized in a static block. private static final

Re: 8000354: (props) Properties.storeToXML/loadFromXML need to allow for alternative implementations

2012-10-06 Thread Jim Gish
Looks good, Alan. A minor typo: For the javadoc of private static class XmlSupport (line 1128 of java/util/Properties.java), probably should say fully qualifed name rather than full-qualified name Jim On 10/05/2012 09:41 AM, Alan Bateman wrote: Properties defines the loadFromXML and

Re: 8000354: (props) Properties.storeToXML/loadFromXML need to allow for alternative implementations

2012-10-05 Thread Mandy Chung
Alan, Nice work and this sets a step toward the future to allow Properties to use a different XML parser implementation. When there is a small footprint parser that Joe is working on, Properties no longer requires JAXP to be present. http://cr.openjdk.java.net/~alanb/8000354/webrev/

Re: 8000354: (props) Properties.storeToXML/loadFromXML need to allow for alternative implementations

2012-10-05 Thread Remi Forax
Hi Alan, just some minor nits, in Properties.XmlSupport, 'provider' should be PROVIDER because it's a constant and instead of using loadProvider(), you should initialize PROVIDER in a static block. line 1172, you declare a provider and line 1174, you initialize it, I think line 1174 should do

Re: 8000354: (props) Properties.storeToXML/loadFromXML need to allow for alternative implementations

2012-10-05 Thread Alan Bateman
On 05/10/2012 19:22, Mandy Chung wrote: Properties.java L1157 - since loadProviderFromProperty method is called within a doPrivileged block, it doesn't seem to be necessary to catch SecurityException. Thanks for the quick review. You're right on catching the SecurityException, this isn't

Re: 8000354: (props) Properties.storeToXML/loadFromXML need to allow for alternative implementations

2012-10-05 Thread Mandy Chung
On 10/5/2012 2:48 PM, Alan Bateman wrote: The updated webrev is here: http://cr.openjdk.java.net/~alanb/8000354/webrev.02/ Looks good. As for the test, I hope that the jdk_util tests do the proper cleanup; meaning that no samevm test sets security manager without resetting it before it