Re: Simple 1-line fix for 8027943 to fix serial version of com.sun.corba.se.spi.orbutil.proxy.CompositeInvocationHandlerImp

2013-11-07 Thread Lance Andersen - Oracle
+1
On Nov 7, 2013, at 4:59 PM, Mandy Chung wrote:

 This reverts 
 com.sun.corba.se.spi.orbutil.proxy.CompositeInvocationHandlerImpl back to the 
 previous serial version.
 
 diff --git 
 a/src/share/classes/com/sun/corba/se/spi/orbutil/proxy/CompositeInvocationHandlerImpl.java
  
 b/src/share/classes/com/sun/corba/se/spi/orbutil/proxy/CompositeInvocationHandlerImpl.java
 --- 
 a/src/share/classes/com/sun/corba/se/spi/orbutil/proxy/CompositeInvocationHandlerImpl.java
 +++ 
 b/src/share/classes/com/sun/corba/se/spi/orbutil/proxy/CompositeInvocationHandlerImpl.java
 @@ -89,4 +89,6 @@
 sm.checkPermission(perm);
 }
 }
 +
 +private static final long serialVersionUID = 4571178305984833743L;
 }
 
 Webrev:
 http://cr.openjdk.java.net/~mchung/jdk8/webrevs/8027943/webrev.00/
 
 Mandy
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: RFR doc only typo in java.io.DataInput

2013-11-07 Thread Lance Andersen - Oracle
+1
On Nov 7, 2013, at 5:29 PM, roger riggs wrote:

 Please review this straightforward typo correction:
 
 Webrev:
   http://cr.openjdk.java.net/~rriggs/webrev-doc-readlong-8024458/
 
 Thanks, Roger
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: JDK RFR to clean-up lint warnings in reflection implementation

2013-11-08 Thread Lance Andersen - Oracle
looks fine Joe
On Nov 8, 2013, at 2:40 PM, Joe Darcy wrote:

 Hello,
 
 Please review the simple patch below which addresses a handful of raw types 
 lint warning in the core reflection implementation code.
 
 (If memory serves, this code dates back from a time during the development of 
 JDK 5 when wildcards could not be used with arrays; before the release 
 shipped, that combination was allowed.)
 
 Thanks,
 
 -Joe
 
 diff -r 46982ca895b4 
 src/share/classes/sun/reflect/generics/reflectiveObjects/ParameterizedTypeImpl.java
 --- 
 a/src/share/classes/sun/reflect/generics/reflectiveObjects/ParameterizedTypeImpl.java
  Fri Nov 08 18:54:29 2013 +
 +++ 
 b/src/share/classes/sun/reflect/generics/reflectiveObjects/ParameterizedTypeImpl.java
  Fri Nov 08 11:37:54 2013 -0800
 @@ -52,7 +52,7 @@
 }
 
 private void validateConstructorArguments() {
 -TypeVariable/*?*/[] formals = rawType.getTypeParameters();
 +TypeVariable?[] formals = rawType.getTypeParameters();
 // check correct arity of actual type args
 if (formals.length != actualTypeArguments.length){
 throw new MalformedParameterizedTypeException();
 diff -r 46982ca895b4 
 src/share/classes/sun/reflect/generics/repository/GenericDeclRepository.java
 --- 
 a/src/share/classes/sun/reflect/generics/repository/GenericDeclRepository.java
  Fri Nov 08 18:54:29 2013 +
 +++ 
 b/src/share/classes/sun/reflect/generics/repository/GenericDeclRepository.java
  Fri Nov 08 11:37:54 2013 -0800
 @@ -42,7 +42,7 @@
 public abstract class GenericDeclRepositoryS extends Signature
 extends AbstractRepositoryS {
 
 -private TypeVariable[] typeParams; // caches the formal type parameters
 +private TypeVariable?[] typeParams; // caches the formal type 
 parameters
 
 protected GenericDeclRepository(String rawSig, GenericsFactory f) {
 super(rawSig, f);
 @@ -64,7 +64,7 @@
  * Return the formal type parameters of this generic declaration.
  * @return the formal type parameters of this generic declaration
  */
 -public TypeVariable/*?*/[] getTypeParameters(){
 +public TypeVariable?[] getTypeParameters(){
 if (typeParams == null) { // lazily initialize type parameters
 // first, extract type parameter subtree(s) from AST
 FormalTypeParameter[] ftps = getTree().getFormalTypeParameters();
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: JDK 8 RFR: more core libs raw warnings cleanup

2013-11-12 Thread Lance Andersen - Oracle
Looks Ok Joe
On Nov 12, 2013, at 4:28 AM, Joe Darcy wrote:

 Hello,
 
 Please review the patch below which would remove another batch of raw type 
 javac lint warnings from the core libraries.
 
 No signatures of public or protected methods in the Java SE specification 
 have been modified. Regression tests in affected areas pass.
 
 Thanks,
 
 -Joe
 
 diff -r 9fcb07df1c92 src/share/classes/java/io/ObjectOutputStream.java
 --- a/src/share/classes/java/io/ObjectOutputStream.javaSat Nov 09 
 04:21:28 2013 +0400
 +++ b/src/share/classes/java/io/ObjectOutputStream.javaTue Nov 12 
 00:51:15 2013 -0800
 @@ -1248,7 +1248,7 @@
 handles.assign(unshared ? null : desc);
 
 Class? cl = desc.forClass();
 -Class[] ifaces = cl.getInterfaces();
 +Class?[] ifaces = cl.getInterfaces();
 bout.writeInt(ifaces.length);
 for (int i = 0; i  ifaces.length; i++) {
 bout.writeUTF(ifaces[i].getName());
 diff -r 9fcb07df1c92 src/share/classes/java/io/ObjectStreamClass.java
 --- a/src/share/classes/java/io/ObjectStreamClass.javaSat Nov 09 04:21:28 
 2013 +0400
 +++ b/src/share/classes/java/io/ObjectStreamClass.javaTue Nov 12 00:51:15 
 2013 -0800
 @@ -1,5 +1,5 @@
 /*
 - * Copyright (c) 1996, 2011, Oracle and/or its affiliates. All rights 
 reserved.
 + * Copyright (c) 1996, 2013, Oracle and/or its affiliates. All rights 
 reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
 @@ -1746,7 +1746,7 @@
 dout.writeUTF(()V);
 }
 
 -Constructor[] cons = cl.getDeclaredConstructors();
 +Constructor?[] cons = cl.getDeclaredConstructors();
 MemberSignature[] consSigs = new MemberSignature[cons.length];
 for (int i = 0; i  cons.length; i++) {
 consSigs[i] = new MemberSignature(cons[i]);
 diff -r 9fcb07df1c92 src/share/classes/java/lang/reflect/Proxy.java
 --- a/src/share/classes/java/lang/reflect/Proxy.javaSat Nov 09 04:21:28 
 2013 +0400
 +++ b/src/share/classes/java/lang/reflect/Proxy.javaTue Nov 12 00:51:15 
 2013 -0800
 @@ -494,9 +494,10 @@
 private final int hash;
 private final WeakReferenceClass?[] refs;
 
 +@SuppressWarnings({rawtypes, unchecked})
 KeyX(Class?[] interfaces) {
 hash = Arrays.hashCode(interfaces);
 -refs = new WeakReference[interfaces.length];
 +refs = (WeakReferenceClass?[])new 
 WeakReference[interfaces.length];
 for (int i = 0; i  interfaces.length; i++) {
 refs[i] = new WeakReference(interfaces[i]);
 }
 diff -r 9fcb07df1c92 src/share/classes/java/nio/file/TempFileHelper.java
 --- a/src/share/classes/java/nio/file/TempFileHelper.javaSat Nov 09 
 04:21:28 2013 +0400
 +++ b/src/share/classes/java/nio/file/TempFileHelper.javaTue Nov 12 
 00:51:15 2013 -0800
 @@ -81,7 +81,7 @@
String prefix,
String suffix,
boolean createDirectory,
 -   FileAttribute[] attrs)
 +   FileAttribute?[] attrs)
 throws IOException
 {
 if (prefix == null)
 @@ -155,7 +155,7 @@
 static Path createTempFile(Path dir,
String prefix,
String suffix,
 -   FileAttribute[] attrs)
 +   FileAttribute?[] attrs)
 throws IOException
 {
 return create(dir, prefix, suffix, false, attrs);
 @@ -167,7 +167,7 @@
  */
 static Path createTempDirectory(Path dir,
 String prefix,
 -FileAttribute[] attrs)
 +FileAttribute?[] attrs)
 throws IOException
 {
 return create(dir, prefix, null, true, attrs);
 diff -r 9fcb07df1c92 src/share/classes/java/util/IdentityHashMap.java
 --- a/src/share/classes/java/util/IdentityHashMap.javaSat Nov 09 04:21:28 
 2013 +0400
 +++ b/src/share/classes/java/util/IdentityHashMap.javaTue Nov 12 00:51:15 
 2013 -0800
 @@ -1243,7 +1243,7 @@
 if (ti = size) {
 throw new ConcurrentModificationException();
 }
 -a[ti++] = (T) new 
 AbstractMap.SimpleEntry(unmaskNull(key), tab[si + 1]);
 +a[ti++] = (T) new 
 AbstractMap.SimpleEntry(unmaskNull(key), tab[si + 1]);
 }
 }
 // fewer elements than expected or concurrent modification from 
 other thread detected
 diff -r 9fcb07df1c92 src/share/classes/java/util/logging/Logger.java
 --- a/src/share/classes/java/util/logging/Logger.javaSat Nov 09 04:21:28 
 2013 +0400
 +++ b/src/share/classes/java/util/logging/Logger.javaTue Nov 12 

Re: RFR(2): 7174936: several String methods claim to always create new String

2013-11-12 Thread Lance Andersen - Oracle
The wording changes seem fine to me.

Thanks for the specdiff as it made it much easier to review


On Nov 12, 2013, at 11:43 AM, Stuart Marks wrote:

 Hi all,
 
 Here's an updated version of the String spec change. Changes from the 
 previous version address comments made by Brent Christian and David Holmes. 
 Specifically:
 
 - Specify copyValueOf() overloads as equivalent to corresponding valueOf() 
 overloads.
 - Remove extranous changes to subSequence() method
 - Clarify wording of concat() method doc.
 
 Bug report:
 
   https://bugs.openjdk.java.net/browse/JDK-7174936
 
 Webrev:
 
   http://cr.openjdk.java.net/~smarks/reviews/7174936/webrev.1/
 
 Specdiff:
 
   
 http://cr.openjdk.java.net/~smarks/reviews/7174936/specdiff.1/overview-summary.html
 
 Thanks!
 
 s'marks
 
 On 11/7/13 2:31 AM, Stuart Marks wrote:
 Hi all,
 
 Please review the following spec changes to java.lang.String.
 
 In several places the specs mention returning new strings. This is
 overspecified; it's sufficient to return a string that satisfies the 
 required
 properties. In some cases the current implementation doesn't create a new 
 string
 -- for example, substring(0) returns 'this' -- which is strictly a violation 
 of
 the spec. The solution is to relax the spec requirement to create new 
 strings.
 
 Also, this change cleans up the specs for the copyValueOf() overloads to make
 them identical to the corresponding valueOf() overloads. Previously, they 
 were
 gratuitously different. I think that some time in the dim, distant past,
 probably before JDK 1.0, they had different semantics, but now they're
 identical. The specs should say they're identical.
 
 This change is spec only, no code changes.
 
 Bug report:
 
 https://bugs.openjdk.java.net/browse/jdk-7174936
 
 Webrev:
 
 http://cr.openjdk.java.net/~smarks/reviews/7174936/webrev.0/
 
 Specdiff:
 
 
 http://cr.openjdk.java.net/~smarks/reviews/7174936/specdiff.0/overview-summary.html
 
 Thanks!
 
 s'marks


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: Review request for 8028234: Remove unused methods in sun.misc.JavaAWTAccess

2013-11-12 Thread Lance Andersen - Oracle
+1
On Nov 12, 2013, at 2:29 PM, Mandy Chung wrote:

 This is a simple code deletion in sun.misc.JavaAWTAccess and its 
 implementation class:
 
 Webrev:
 http://cr.openjdk.java.net/~mchung/jdk8/webrevs/8028234/webrev.00/
 
 This patch removes the methods from sun.misc.JavaAWTAccess that are no longer 
 used.  The only dependency remaining from core-libs to AppContext is the 
 ability to obtain an opaque unique object to represent an applet context ifit 
 is running in an applet environment.
 
 thanks
 Mandy


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: RFR (JAXP): 8028111 : XML readers share the same entity expansion counter

2013-11-13 Thread Lance Andersen - Oracle
looks fine joe
On Nov 13, 2013, at 3:02 PM, huizhe wang wrote:

 Hi,
 
 The issue is that the limits applied to each processing process rather than 
 each file processing. This applies to not only StAX as reported, but also 
 other parsers and validators. The fix is to add reset to XMLSecurityManager 
 and call it upon each file processing. XSLT Transform is verified fixed as 
 the underlying parsers are fixed.
 
 webrev:
 http://cr.openjdk.java.net/~joehw/jdk8/8028111/webrev/
 
 Thanks,
 Joe
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: RFR: JDK-8028631 - Improve the test coverage to the pathname handling on unix-like platforms

2013-11-19 Thread Lance Andersen - Oracle
The changes seem OK.  I did not run the tests though
On Nov 19, 2013, at 2:08 PM, Dan Xu wrote:

 Hi All,
 
 We have java/io/pathNames/GeneralWin32.java testcase to do the general 
 exhaustive test of pathname handling on windows. I am adding a new test 
 GeneralSolaris.java to test the pathname handling in unix-like platforms. In 
 the changes, I also make sure the test can run successfully in concurrency 
 mode. Please help review it. Thanks!
 
 Bug: https://bugs.openjdk.java.net/browse/JDK-8028631
 Webrev: http://cr.openjdk.java.net/~dxu/8028631/webrev/
 
 -Dan


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: RFR: 8028638: java/rmi/activation/Activatable/checkRegisterInLog/CheckRegisterInLog.java fails

2013-11-19 Thread Lance Andersen - Oracle
looks fine
On Nov 19, 2013, at 5:24 PM, Stuart Marks wrote:

 Hi all,
 
 Please review this small fix for an intermittent timeout. Nothing seems to be 
 going wrong except that if the machine running the test is exceptionally 
 slow, spurious timeouts will occur. The solution is to raise the timeout in 
 the RMI test infrastructure.
 
 Bug:
   https://bugs.openjdk.java.net/browse/JDK-8028638
 
 Diff:
   (appended below)
 
 Thanks,
 
 s'marks
 
 
 # HG changeset patch
 # User smarks
 # Date 1384899795 28800
 # Node ID ebbfb1b45a4e6b37d339942568a662268dcb18fe
 # Parent  67d742c759717ca17518aaadb17725cac85c5897
 8028638: 
 java/rmi/activation/Activatable/checkRegisterInLog/CheckRegisterInLog.java 
 fails
 Reviewed-by: XXX
 
 diff -r 67d742c75971 -r ebbfb1b45a4e test/java/rmi/testlibrary/RMID.java
 --- a/test/java/rmi/testlibrary/RMID.java Tue Nov 19 20:10:58 2013 +0100
 +++ b/test/java/rmi/testlibrary/RMID.java Tue Nov 19 14:23:15 2013 -0800
 @@ -74,6 +74,10 @@
 // +
 //  -Djava.security.debug=all ;
 
 +// Set execTimeout to 60 sec (default is 30 sec)
 +// to avoid spurious timeouts on slow machines.
 +options +=  -Dsun.rmi.activation.execTimeout=6;
 +
 return options;
 }
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: RFR (JAXP) 8028822 : Error in the documentation for newFactory method of the javax.xml.stream factories

2013-11-26 Thread Lance Andersen - Oracle
looks OK Joe
On Nov 26, 2013, at 5:27 PM, huizhe wang wrote:

 
 On 11/26/2013 1:59 PM, roger riggs wrote:
 Hi,
 
 I looked at that twice also.java.time had a similar situation.
 
 To get to the TCCL you need to call ServiceLoader.load(type).
 
 The FactoryFinder:348 findServiceProvider method should test if cl == null 
 and call the original ServiceLoader function.
 
 Yes. I forgot that part. load(service, loader) treats null as the system 
 class loader was one of the topics in the discussion of the spec.
 
 Joe
 
 
 Roger
 
 On 11/26/2013 4:37 PM, Alan Bateman wrote:
 On 26/11/2013 18:38, huizhe wang wrote:
 Hi all,
 
 Here's revised webrev, as Alan suggested, including the implementation.
 
 http://cr.openjdk.java.net/~joehw/jdk8/jaxp16MR/8028822/webrev/
 So is this handling the null case correctly? It is spec'ed to use the TCCL 
 but it looks like it's going through to ServiceLoader.load where null means 
 the system class loader. Maybe this needs to wait for tests as that it 
 really the only way to check this.
 
 -Alan
 
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Review request 8029417: Minor javadoc cleanup for JDBC 4.2

2013-12-02 Thread Lance Andersen - Oracle
Hi all

This is a review request for some minor javadoc clarifications for JDBC 4.2 
based on some feedback that I received.

The webrev can be found http://cr.openjdk.java.net/~lancea/8029417/webrev.00

Best
Lance

Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: RFR: 8029489: StringJoiner spec for setEmptyValue() and length() is malformatted

2013-12-03 Thread Lance Andersen - Oracle
looks OK
On Dec 3, 2013, at 8:40 PM, Stuart Marks wrote:

 Hi all,
 
 Please review the following small javadoc change. The StringJoiner doc for a 
 couple methods uses i.e. in the first sentence, which screws up the javadoc 
 logic that pulls the first sentence into the Method Summary. This is an 
 editorial change to fix this up; there is no actual specification change.
 
 Thanks!
 
 s'marks
 
 # HG changeset patch
 # User smarks
 # Date 1386121046 28800
 # Node ID 0bc91b9f6afd9b4dceea31ecd1036e367b365690
 # Parent  75165f6c1c505ff43f7fd235a95b2e7955413b78
 8029489: StringJoiner spec for setEmptyValue() and length() is malformatted
 Reviewed-by: XXX
 
 diff -r 75165f6c1c50 -r 0bc91b9f6afd 
 src/share/classes/java/util/StringJoiner.java
 --- a/src/share/classes/java/util/StringJoiner.java   Tue Dec 03 17:25:28 
 2013 -0800
 +++ b/src/share/classes/java/util/StringJoiner.java   Tue Dec 03 17:37:26 
 2013 -0800
 @@ -131,7 +131,7 @@
 /**
  * Sets the sequence of characters to be used when determining the string
  * representation of this {@code StringJoiner} and no elements have been
 - * added yet, i.e. when it is empty.  A copy of the {@code emptyValue}
 + * added yet, that is, when it is empty.  A copy of the {@code 
 emptyValue}
  * parameter is made for this purpose. Note that once an add method has 
 been
  * called, the {@code StringJoiner} is no longer considered empty, even if
  * the element(s) added correspond to the empty {@code String}.
 @@ -228,8 +228,8 @@
 }
 
 /**
 - * The length of the {@code StringJoiner} value, i.e. the length of
 - * {@code String} representation of the {@code StringJoiner}. Note that 
 if
 + * Returns the length of the {@code String} representation
 + * of this {@code StringJoiner}. Note that if
  * no add methods have been called, then the length of the {@code String}
  * representation (either {@code prefix + suffix} or {@code emptyValue})
  * will be returned. The value should be equivalent to


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: RFR: JDK-8028712 : Tidy warnings cleanup for java.sql package

2013-12-05 Thread Lance Andersen - Oracle
Hi Serge



This looks OK.



For

--- old/src/share/classes/java/sql/package.html 2013-12-05 15:08:50.587885460 
+
+++ new/src/share/classes/java/sql/package.html 2013-12-05 15:08:50.435885464 
+

Please remove the following


Package Specification

• Specification of the JDBC 4.0 API
Related Documentation

• Getting Started--overviews of the major interfaces
• Chapters on the JDBC API--from the online version of The Java 
Tutorial Continued
• JDBCTMAPI Tutorial and Reference, Third Edition-- a complete 
reference and tutorial for the JDBC 3.0 API



The above links keep breaking now that we are off of java.sun.com

And I agree with roger, please use br


Alan, yes I can look to clean up some of the formatting crud for Java SE 9 once 
we have access to the workspace

Thank you for doing this Serge.

Best
Lance
On Dec 5, 2013, at 10:31 AM, Alan Bateman wrote:

 On 05/12/2013 17:25, Serge wrote:
 Hi all,
 please review the fix
 http://cr.openjdk.java.net/~yan/8028712/webrev.02/ 
 http://cr.openjdk.java.net/%7Eyan/8028712/webrev.02/
 for
 https://bugs.openjdk.java.net/browse/JDK-8028712
 
 This patch cleanup tidy warnings for generated html documentation, and do
 not affect the appearance of the documentation.
 The removal of the p tags seems okay. The only thing that I'm not sure 
 about the addition of br/ to the package docs (is that needed?).
 
 Lance - while scanning this patch then it appears that some of the formatting 
 in the javadoc comments is all over the place (java.sql.Connection is one 
 example). It might be good to clean that up once the jdk9 project is open for 
 business.
 
 -Alan
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: RFR (JAXP) 8029895 : XMLOutputFactory.newFactory(String, ClassLoader) - incorrect specification

2013-12-11 Thread Lance Andersen - Oracle
looks fine joe
On Dec 11, 2013, at 4:10 PM, huizhe wang wrote:

 
 On 12/11/2013 12:21 PM, Alan Bateman wrote:
 On 11/12/2013 19:52, huizhe wang wrote:
 Hi,
 
 This is a quick documentation change to fix an error in 
 javax.xml.stream.XMLOutputFactory:
 
 http://cr.openjdk.java.net/~joehw/jdk8/jaxp16MR/8029895/webrev/
 
 Thanks,
 Joe
 It looks okay although I think it could be reduced to something like The 
 original method was incorrectly defined to return XMLInputFactory.
 
 Updated to use your statement:
 
 http://cr.openjdk.java.net/~joehw/jdk8/jaxp16MR/8029895/webrev/
 
 -Joe
 
 
 -Alan.
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: RFR (JAXP) 8029955 : AIOB in XMLEntityScanner.scanLiteral upon parsing literals with 100 LF chars

2013-12-17 Thread Lance Andersen - Oracle
Joe,

I thought this looked OK also


On Dec 17, 2013, at 12:26 PM, huizhe wang wrote:

 
 On 12/17/2013 4:10 AM, Daniel Fuchs wrote:
 Hi Joe,
 
 The fix looks good - though I wonder at whether incrementing
 whiteSpaceLookup by a fix amount wouldn't be better than
 doubling its length.
 
 Both would be okay. The case, as demonstrated in the report, that there are 
 hundreds of LFs in an attribute, is very rare.
 
 Best,
 Joe
 
 
 best regards,
 
 -- daniel
 
 On 12/16/13 8:31 PM, huizhe wang wrote:
 Hi,
 
 This is a quick fix for a whitespace buffer that was not adjusted
 properly in one of the two cases.  The buffer, whiteSpaceLookup, is
 filled in two cases and adjusted properly the 2nd time. The code is
 moved into a method storeWhiteSpace so that it's shared for the 1st case
 as well.
 
 Note at line 1175, there is no need to save character 0x20 since all
 whitespace characters will later be replaced with character 0x20.
 
 webrevs:
 http://cr.openjdk.java.net/~joehw/jdk8/8029955/webrev/
 
 Thanks,
 Joe
 
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: RFR: JDK-8028712 : Tidy warnings cleanup for java.sql package

2014-01-02 Thread Lance Andersen - Oracle
Hi Serge,
On Dec 20, 2013, at 9:16 AM, Serge wrote:

 Hi all.
 
 Please review a second fix
 http://cr.openjdk.java.net/~yan/8028712/webrev.03/
 
 for
 https://bugs.openjdk.java.net/browse/JDK-8028712
 
 I deleted part of java/sql/package.html,

I did not see the section below deleted, perhaps the change did not get pushed 
to where you generated the webrev from?
 and replaced br/ to br for compliance with html 3.2
 
 
 On 12/05/2013 10:39 PM, Lance Andersen - Oracle wrote:
 Hi Serge
 
 
 
 This looks OK.
 
 
 
 For
 
 --- old/src/share/classes/java/sql/package.html  2013-12-05 
 15:08:50.587885460 +
 +++ new/src/share/classes/java/sql/package.html  2013-12-05 
 15:08:50.435885464 +
 
 Please remove the following
 
 
 Package Specification
 
  . Specification of the JDBC 4.0 API
 Related Documentation
 
  . Getting Started--overviews of the major interfaces
  . Chapters on the JDBC API--from the online version of The Java 
 Tutorial Continued
  . JDBCTMAPI Tutorial and Reference, Third Edition-- a complete 
 reference and tutorial for the JDBC 3.0 API
 
 
 
 The above links keep breaking now that we are off of java.sun.com
 
 And I agree with roger, please use br
 
 
 Alan, yes I can look to clean up some of the formatting crud for Java SE 9 
 once we have access to the workspace
 
 Thank you for doing this Serge.
 
 Best
 Lance
 On Dec 5, 2013, at 10:31 AM, Alan Bateman wrote:
 
 On 05/12/2013 17:25, Serge wrote:
 Hi all,
 please review the fix
 http://cr.openjdk.java.net/~yan/8028712/webrev.02/ 
 http://cr.openjdk.java.net/%7Eyan/8028712/webrev.02/
 for
 https://bugs.openjdk.java.net/browse/JDK-8028712
 
 This patch cleanup tidy warnings for generated html documentation, and do
 not affect the appearance of the documentation.
 The removal of the p tags seems okay. The only thing that I'm not sure 
 about the addition of br/ to the package docs (is that needed?).
 
 Lance - while scanning this patch then it appears that some of the 
 formatting in the javadoc comments is all over the place 
 (java.sql.Connection is one example). It might be good to clean that up 
 once the jdk9 project is open for business.
 
 -Alan
 
 
 
 Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
 Oracle Java Engineering
 1 Network Drive
 Burlington, MA 01803
 lance.ander...@oracle.com
 
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: RFR java.time.Duration spec correction (8031103)

2014-01-06 Thread Lance Andersen - Oracle
looks fine Roger
On Jan 6, 2014, at 2:09 PM, roger riggs wrote:

 Please review this minor specification correction to the 
 java.time.Duration.toDays() and
 toHours() methods.  Only the javadoc is corrected, no code or tests are 
 affected.
 
 Webrev:
 http://cr.openjdk.java.net/~rriggs/webrev-duration-javadoc-8031103/
 
 Thanks, Roger
 
 JDK-8031103 https://bugs.openjdk.java.net/browse/JDK-8031103 
 java.time.Duration has wrong Javadoc Comments in toDays() and toHours()
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: JDK 9 RFR of JDK-8027063 SecurityManger.getClassContext returns a raw type

2014-01-06 Thread Lance Andersen - Oracle
+1
On Jan 6, 2014, at 3:53 PM, Joe Darcy wrote:

 Hello,
 
 Please review the simple change to fix JDK-8027063 
 SecurityManger.getClassContext returns a raw type, which changes a signature 
 of a protected method in SecurityManger to remove a use of raw types in the 
 core libraries:
 
 --- a/src/share/classes/java/lang/SecurityManager.javaMon Jan 06 11:48:32 
 2014 -0800
 +++ b/src/share/classes/java/lang/SecurityManager.javaMon Jan 06 12:51:52 
 2014 -0800
 @@ -1,5 +1,5 @@
 /*
 - * Copyright (c) 1995, 2011, Oracle and/or its affiliates. All rights 
 reserved.
 + * Copyright (c) 1995, 2014, Oracle and/or its affiliates. All rights 
 reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
 @@ -307,7 +307,7 @@
  *
  * @return  the execution stack.
  */
 -protected native Class[] getClassContext();
 +protected native Class?[] getClassContext();
 
 /**
  * Returns the class loader of the most recently executing method from
 
 A clean build succeeds and the SecurityManager tests pass after this change.
 
 Thanks,
 
 -Joe


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: JDK 9 RFR of JDK-8031210 Remove serial warning from java.lang.Enum

2014-01-06 Thread Lance Andersen - Oracle
+1
On Jan 6, 2014, at 4:41 PM, Joe Darcy wrote:

 Hello,
 
 Please review the patch below to add a @SuppressWarning(serial) to 
 java.lang.Enum to resolve a lint warning in the core libraries.
 
 Thanks,
 
 -Joe
 
 --- a/src/share/classes/java/lang/Enum.javaMon Jan 06 11:48:32 2014 -0800
 +++ b/src/share/classes/java/lang/Enum.javaMon Jan 06 13:37:06 2014 -0800
 @@ -1,5 +1,5 @@
 /*
 - * Copyright (c) 2003, 2011, Oracle and/or its affiliates. All rights 
 reserved.
 + * Copyright (c) 2003, 2014, Oracle and/or its affiliates. All rights 
 reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
 @@ -52,6 +52,8 @@
  * @see java.util.EnumMap
  * @since   1.5
  */
 +@SuppressWarnings(serial) // No serialVersionUID needed due to
 +// special-casing of enum types.
 public abstract class EnumE extends EnumE
 implements ComparableE, Serializable {
 /**
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: RFR: JDK-8028726 - (prefs) Check src/solaris/native/java/util/FileSystemPreferences.c for JNI pending exceptions

2014-01-06 Thread Lance Andersen - Oracle
Dan,

Looks OK, but line 914 which you did not change, notice the comments not sure 
if that is common in this code but seemed a bit off to me:

914 //// If at first, you don't succeed...
On Jan 6, 2014, at 5:29 PM, Dan Xu wrote:

 Hi All,
 
 Please review the simple fix for JNI pending exceptions in 
 FileSystemPreferences.c. Thanks!
 
 Bug: https://bugs.openjdk.java.net/browse/JDK-8028726
 Webrev: http://cr.openjdk.java.net/~dxu/8028726/webrev/
 
 -Dan


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: JDK 9 RFR of raw type lint fix to java.lang.management

2014-01-07 Thread Lance Andersen - Oracle
+1
On Jan 7, 2014, at 3:30 PM, Joe Darcy wrote:

 Hello,
 
 Please review another minor lint fix of a raw type issues in the core 
 libraries:
 
diff -r 2647b91dbc2a 
 src/share/classes/java/lang/management/ManagementFactory.java
 --- a/src/share/classes/java/lang/management/ManagementFactory.java Tue Jan 
 07 09:58:16 2014 -0800
 +++ b/src/share/classes/java/lang/management/ManagementFactory.java Tue Jan 
 07 12:29:22 2014 -0800
 @@ -1,5 +1,5 @@
 /*
 - * Copyright (c) 2003, 2013, Oracle and/or its affiliates. All rights 
 reserved.
 + * Copyright (c) 2003, 2014, Oracle and/or its affiliates. All rights 
 reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
 @@ -612,7 +612,7 @@
  is not an instance of  + mxbeanInterface);
 }
 
 -final Class[] interfaces;
 +final Class?[] interfaces;
 // check if the registered MBean is a notification emitter
 boolean emitter = connection.isInstanceOf(objName, NOTIF_EMITTER);
 
 
 Thanks,
 
 -Joe


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: JDK 9 RFR of JDK-8031369: Fix raw types warnings in sun.misc.{Cache, SoftCache}

2014-01-07 Thread Lance Andersen - Oracle
looks good Joe
On Jan 7, 2014, at 6:58 PM, Joe Darcy wrote:

 Hello,
 
 Please review the fix below to address
 
JDK-8031369: Fix raw types warnings in sun.misc.{Cache, SoftCache}
 
 by a quick-and-dirty generification and deprecation of some very old classes
 
http://cr.openjdk.java.net/~darcy/8031369.0/
 
 Corresponding patch below.
 
 In the fullness of time, these classes should probably be removed from the 
 platform, but for the moment I'm more concerned with eliminating the several 
 dozen lint warnings in this code.
 
 Thanks,
 
 -Joe
 
 --- old/src/share/classes/sun/misc/Cache.java2014-01-07 
 15:51:32.0 -0800
 +++ new/src/share/classes/sun/misc/Cache.java2014-01-07 
 15:51:32.0 -0800
 @@ -1,5 +1,5 @@
 /*
 - * Copyright (c) 1995, 1996, Oracle and/or its affiliates. All rights 
 reserved.
 + * Copyright (c) 1995, 2014, Oracle and/or its affiliates. All rights 
 reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
 @@ -74,8 +74,9 @@
  * @see java.lang.Object#equals
  * @see sun.misc.Ref
  */
 +@Deprecated
 public
 -class Cache extends Dictionary {
 +class Cache extends DictionaryObject, Object {
 /**
  * The hash table data.
  */
 @@ -163,7 +164,7 @@
  * @see Cache#elements
  * @see Enumeration
  */
 -public synchronized Enumeration keys() {
 +public synchronized EnumerationObject keys() {
 return new CacheEnumerator(table, true);
 }
 
 @@ -173,7 +174,7 @@
  * @see Cache#keys
  * @see Enumeration
  */
 -public synchronized Enumeration elements() {
 +public synchronized EnumerationObject elements() {
 return new CacheEnumerator(table, false);
 }
 
 @@ -305,7 +306,7 @@
  * A Cache enumerator class.  This class should remain opaque
  * to the client. It will use the Enumeration interface.
  */
 -class CacheEnumerator implements Enumeration {
 +class CacheEnumerator implements EnumerationObject {
 boolean keys;
 int index;
 CacheEntry table[];
 --- old/src/share/classes/sun/misc/SoftCache.java2014-01-07 
 15:51:33.0 -0800
 +++ new/src/share/classes/sun/misc/SoftCache.java2014-01-07 
 15:51:33.0 -0800
 @@ -1,5 +1,5 @@
 /*
 - * Copyright (c) 1998, 2006, Oracle and/or its affiliates. All rights 
 reserved.
 + * Copyright (c) 1998, 2014, Oracle and/or its affiliates. All rights 
 reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
 @@ -101,8 +101,8 @@
  * @see java.lang.ref.SoftReference
  */
 
 -
 -public class SoftCache extends AbstractMap implements Map {
 +@Deprecated
 +public class SoftCache extends AbstractMapObject, Object implements 
 MapObject, Object {
 
 /* The basic idea of this implementation is to maintain an internal 
 HashMap
that maps keys to soft references whose referents are the keys' values;
 @@ -115,18 +115,18 @@
  */
 
 
 -static private class ValueCell extends SoftReference {
 +static private class ValueCell extends SoftReferenceObject {
 static private Object INVALID_KEY = new Object();
 static private int dropped = 0;
 private Object key;
 
 -private ValueCell(Object key, Object value, ReferenceQueue queue) {
 +private ValueCell(Object key, Object value, ReferenceQueueObject 
 queue) {
 super(value, queue);
 this.key = key;
 }
 
 private static ValueCell create(Object key, Object value,
 -ReferenceQueue queue)
 + ReferenceQueueObject queue)
 {
 if (value == null) return null;
 return new ValueCell(key, value, queue);
 @@ -154,10 +154,10 @@
 
 
 /* Hash table mapping keys to ValueCells */
 -private Map hash;
 +private MapObject, Object hash;
 
 /* Reference queue for cleared ValueCells */
 -private ReferenceQueue queue = new ReferenceQueue();
 +private ReferenceQueueObject queue = new ReferenceQueue();
 
 
 /* Process any ValueCells that have been cleared and enqueued by the
 @@ -189,7 +189,7 @@
  *   factor is less than zero
  */
 public SoftCache(int initialCapacity, float loadFactor) {
 -hash = new HashMap(initialCapacity, loadFactor);
 +hash = new HashMap(initialCapacity, loadFactor);
 }
 
 /**
 @@ -202,7 +202,7 @@
  *   or equal to zero
  */
 public SoftCache(int initialCapacity) {
 -hash = new HashMap(initialCapacity);
 +hash = new HashMap(initialCapacity);
 }
 
 /**
 @@ -210,7 +210,7 @@
  * capacity and the default load factor.
  */
 public SoftCache() {
 -hash = new HashMap();
 +hash = new HashMap();
 }
 
 
 @@ -348,13 +348,13 @@
 /* Internal class for 

Re: RFR JDK 9: 8032502: java.time add @param tags to readObject

2014-01-22 Thread Lance Andersen - Oracle
looks fine Roger as  am sure this will make the doclint warnings less 
On Jan 22, 2014, at 4:26 PM, roger riggs wrote:

 Please review this javadoc improvement to add @param tags to readObject
 
 Webrev:
 
   http://cr.openjdk.java.net/~rriggs/webrev-time-param-8032502/
 
 Thanks, Roger
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: RFR (JAXP): 8032392 : Spec: javax.xml.stream.XMLEventFactory/XMLOutputFactory/XMLInputFactory.newFactory(String, ClassLoader) referring to ServiceLoader.load(Class, ClassLoader)

2014-01-24 Thread Lance Andersen - Oracle
+1
On Jan 24, 2014, at 3:31 PM, huizhe wang wrote:

 Hi,
 
 Please review a javadoc change to javax.xml.stream factories. This change 
 makes it clear that the two args ServiceLoader#load method is used when the 
 specified classLoader is not null.
 
 http://cr.openjdk.java.net/~joehw/jdk9/8032392/webrev/
 
 Thanks,
 Joe
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: RFR 8032221 Typo in java.util.date

2014-01-31 Thread Lance Andersen - Oracle
looks fine.  getting rid of tt and code, is something  I guess we  should 
look to do throughout all of our code?
On Jan 31, 2014, at 1:33 PM, roger riggs wrote:

 Please review a typo and javadoc cleanup for java.util.Date
 
 webrev:
  http://cr.openjdk.java.net/~rriggs/webrev-date-typo-8032221/
 
 Thanks, Roger
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: Review request for JDK-8030010: cleanup native code warnings

2014-02-12 Thread Lance Andersen - Oracle
Hi Mandy

This looks OK to me

On Feb 12, 2014, at 12:46 PM, Mandy Chung wrote:

 This patch cleans up a few trivial native warnings (mainly remove local 
 unreferenced variable)
   https://bugs.openjdk.java.net/browse/JDK-8030010
 
 Webrev at:
 http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8030010/webrev.00/
 
 This patch was contributed by Francis Andre [1] and I fixed a couple other 
 trivial ones.
 
 Mandy
 [1] http://mail.openjdk.java.net/pipermail/build-dev/2013-December/011332.html


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: RFR [4682009] Typo in javadocs in javax/naming

2014-02-14 Thread Lance Andersen - Oracle
Looks fine.  assume the code/code will be addressed as part of a full sweep 
of javax/naming
On Feb 14, 2014, at 2:48 PM, Ivan Gerasimov wrote:

 Hello!
 
 May I please have a review of the fix?
 
 It's not meant to be a proof reading, I only fixed some obvious typos.
 Some of them were reported at the time of java 1.4, so it is probably the 
 time to have them fixed.
 
 BUG: https://bugs.openjdk.java.net/browse/JDK-4682009
 WEBREV: http://cr.openjdk.java.net/~igerasim/4682009/0/webrev/
 
 Sincerely yours,
 Ivan


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: RFR 9: 8032491: DateTimeFormatter fixed width adjacent value parsing does not match spec

2014-02-28 Thread Lance Andersen - Oracle
Looks Ok.  Kind of surprised the tck tests have no assertion details in the 
tests.  Minor nit would have been nice to have even a minor comment for the new 
method DateTimeFormatterBuilder though that seems to be the norm in some 
scenarios for the smaller methods.


On Feb 28, 2014, at 4:48 PM, roger riggs wrote:

 Please review this bug fix from Steve Colebourne for java.time parsing of
 fixed width adjacent values. The patch includes new tests.
 
 Webrev:
  http://cr.openjdk.java.net/~rriggs/webrev-parse-adjacent-8032491/
 
 Thanks, Roger
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: RFR (JAXP): 8035577: Xerces Update: impl/xpath/regex/RangeToken.java

2014-03-05 Thread Lance Andersen - Oracle

On Mar 5, 2014, at 3:37 PM, Xueming Shen wrote:

 nitpicking,
 
 (1) shouldn't the variable at #468 to be updated to lch instead of uch as 
 well?

I would agree given you are now calling Character.toLowerCase
 (2) StringBuilder can be used to replace the StringBuffer in toString().
Agree, but I think this could be considered optional for this putback...


 
 -Sherman
 
 On 03/05/2014 12:18 PM, David Li wrote:
 Hi,
 
 This is an update from Xerces for file impl/xpath/regex/TokenRange.java.  
 For details, please refer to: 
 https://bugs.openjdk.java.net/browse/JDK-8035577.
 
 Webrevs: http://cr.openjdk.java.net/~joehw/jdk9/8035577/webrev/
 
 No new tests.  There were none added in Xerces.
 
 Existing tests: JAXP SQE and unit tests passed.
 
 Thanks,
 David
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: RFR (JAXP): 8035577: Xerces Update: impl/xpath/regex/RangeToken.java

2014-03-05 Thread Lance Andersen - Oracle

On Mar 5, 2014, at 5:10 PM, huizhe wang wrote:

 
 On 3/5/2014 12:46 PM, Lance Andersen - Oracle wrote:
 On Mar 5, 2014, at 3:37 PM, Xueming Shen wrote:
 
 nitpicking,
 
 (1) shouldn't the variable at #468 to be updated to lch instead of uch 
 as well?
 I would agree given you are now calling Character.toLowerCase
 
 Also, at line 463 and 464, it looks like 'uppers' meant to be 'lowers' 
 defined at 462.

I think we should add tests with this change it looks like we really need to 
test this if your suggestion above is correct.  
 
 (2) StringBuilder can be used to replace the StringBuffer in toString().
 Agree, but I think this could be considered optional for this putback...
 
 Yes, feel free to replace StringBuffer with StringBuilder, Vector to 
 ArrayList and etc.
 
 -Joe
 
 
 
 -Sherman
 
 On 03/05/2014 12:18 PM, David Li wrote:
 Hi,
 
 This is an update from Xerces for file impl/xpath/regex/TokenRange.java.  
 For details, please refer to: 
 https://bugs.openjdk.java.net/browse/JDK-8035577.
 
 Webrevs: http://cr.openjdk.java.net/~joehw/jdk9/8035577/webrev/
 
 No new tests.  There were none added in Xerces.
 
 Existing tests: JAXP SQE and unit tests passed.
 
 Thanks,
 David
 
 
 Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
 Oracle Java Engineering
 1 Network Drive
 Burlington, MA 01803
 lance.ander...@oracle.com
 
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: RFR (JAXP): 8035577: Xerces Update: impl/xpath/regex/RangeToken.java

2014-03-20 Thread Lance Andersen - Oracle
I think this OK.

The comments with the o--o did not do much for me though and found them a bit 
confusing but perhaps I need more coffee this morning ?

Also, not sure we need the @author tag but I think its usage varies in the 
workspace

Best
Lance
On Mar 19, 2014, at 7:10 PM, David Li wrote:

 Hi,
 
 This is an update from Xerces for file impl/xpath/regex/TokenRange.java.  For 
 details, please refer to: https://bugs.openjdk.java.net/browse/JDK-8035577.
 
 Webrevs: http://cr.openjdk.java.net/~joehw/jdk9/8035577/webrev/
 
 Existing tests: JAXP SQE and unit tests passed.
 
 Test cases added for typo fix in RangeToken.intersectRanges.  Code also 
 updated to fix a bug where regular expression intersection returns incorrect 
 value when first range ends later than second range.   Example below. Test 
 cases have been added to cover any scenarios that the code changes affect.
 
 new RegularExpression((?[b-d][a-r])); - returns [b-d] (Correct)
 new RegularExpression((?[a-r][b-d])); - returns [b-de-r] (Incorrect)
 
 Thanks,
 David
 
 P.S. Notes on bug fixes.
 1) Line 404 removal of while loop.
 This fixes a new bug where incorrect results are given when first range ends 
 later than second range.  In the old code we got
 (?[a-r][b-d]) - returns [b-de-r]
 By removing the while loop, we get [b-d].
 This while loop looks like a copy-paste error from subtractRanges. In 
 subtractRanges we need to keep the leftover portion from the first range, but 
 this does not apply to intersection.
 
 2) Line 388, addition of src2 += 2;
 This code change affects anything of the form (?[a-r][b-eg-j]).  The code 
 execution is diagrammed below.
 oo  (src1)
  o--o o--o (src2)
 For the first match we get
 oo  (src1)
  o--o  (src2)
 Next we want to run src2+=2 to get the second pair of endpoints (since the 
 first two endpoints are already used).  Notice how src1begin has been updated 
 to this.ranges[src1] = src2end+1, which is directly from the code.
  o--o  (src1)
   o--o (src2)
 The src2+=2 statement was left out of the old code, and is added in this 
 webrev.  If we leave out the src2+=2 at line 388, on the next iteration of 
 the large while loop we will reach case } else if (src2end  src1begin) { 
 which also executes src2+=2.  This means the correct final result is 
 generated, but on a later loop. We want to add the new code because it's 
 better to have all associated variable updated in the sameloop.  In addition, 
 all the other conditions have similar src1 or src2 updates.
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: RFR : 7162902 / 6893617 Umbrella port of a number of corba bug fixes from JDK 6 to jdk7u/8

2012-06-27 Thread Lance Andersen - Oracle
Hi Sean,

I think this looks good.  ship it :-)

Best
Lance
On Jun 25, 2012, at 4:26 PM, Sean Coffey wrote:

 Hi,
 
 I'm looking for a code review around the following corba changes. It turns 
 out that we've a few bug fixes in corba area for jdk6 that were never forward 
 ported to jdk7 or 8. The port is pretty much identical to what was used in 
 JDK6. Some formatting and diamond operator changes introduced but that's 
 about it. There were a few bug fixes to follow up on regressions around the 
 initial fix but this umbrella port should cover all issues.
 
 The initial bugs fixes were fixed under the jets category in the bug tool 
 and that category entered read only mode over a year ago. Hence the 
 requirement for a new bug ID.
 
 The main bug fix is 6725987 which is an issue where references to the ORB 
 were being kept after ORB.destroy was called.
 
 http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7162902
 An accompanying CNCtx fix in JDK repo is also made to correct an issue where 
 the java.naming.corba.orb system property wasn't used correctly. (CR 6893617)
 http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6893617
 
 Corba testsuites run and no issues seen. I've also run some JCK testing and 
 saw no issues.
 
 http://cr.openjdk.java.net/~coffeys/webrev.7162902.jdk8/ (corba)
 http://cr.openjdk.java.net/~coffeys/webrev.6893617.jdk8/ (jdk)
 
 The corba codebase code formatting seems to vary quite a bit. I've 
 reformatted in a small number of areas but the whole corba repo probably 
 warrants it's own clean up project at a later date.
 
 regards,
 Sean.


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: javax.sql.rowset.serial.SerialBlob doesn't support free and getBinaryStream

2012-07-02 Thread Lance Andersen - Oracle
Hi Deven,

Thanks for the email and the proposed patch.  I will look at this later today 
or tomorrow.  I actually have made these changes in my workspace for JDK 8 but 
will compare your changes to mine.

Best
Lance
On Jul 2, 2012, at 5:04 AM, Deven You wrote:

 Hi All,
 Could anyone notice this problem?
 
 Thanks a lot!
 On 06/25/2012 04:18 PM, Deven You wrote:
 Hi All,
 
 First of all, if the jdbc problem has a better mailing list to post please 
 tell me.
 
 I find that javax.sql.rowset.serial.SerialBlob is not fully implemented in 
 OpenJDK 8. Methods
 
public InputStream getBinaryStream(long pos,long length) throws 
 SQLException
public void free() throws SQLException
 
 only throw UnsupportedOperationException.
 
 I have made a patch[1] to implement these 2 methods. Could anyone take a 
 look to review it.
 
 BTW: I think the spec for SerialBlob is not very clear like it doesn't 
 mention if all method rather than free() need throw any exception after 
 free() is invoked. However that behavior seems reasonable.
 
 [1] http://cr.openjdk.java.net/~littlee/OJDK-576/webrev
 
 Thanks a lot.
 
 
 
 -- 
 Best Regards,
 
 Deven
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: javax.sql.rowset.serial.SerialBlob doesn't support free and getBinaryStream

2012-07-05 Thread Lance Andersen - Oracle
Hi Deven,

We had a holiday here on Weds,  so I am still catching up.

First, thank you for taking the time to propose a patch.

 Here are a few comments based on the changes I have made in my workspace and  
comparing to your proposed changes 

- The same issue applies to SerialClob
- instead of duplicating all of the code to check if free has been called in 
each method, I created a private method which does the validation
- I do not see a reason to make the flag isFree transient, if the object has 
been freed, it has been freed
- free() needs to call blob.free if the blob object is not null prior to 
nulling out the object
- Your 2nd if statement in getBinaryStream() duplicates part of the check in 
the 1st if statement
- All of the public methods need to have their javadocs indicate that if called 
after free has been called, a SerialException will be thrown.  additional calls 
to free is a no-op
- The test case is is a good start.I would change this a bit though:
  + create separate test classes for free and getbinarystream
  + each individual test  be a method
  + If you catch the expected Exception, print that the test has passed
  + Each test needs a comment as to what it is trying to validate (just a 
simple comment is fine)
  + return 0 if the test passed, 1 if it fails  and then print the number of 
failed tests at the end after running through all of them


As I need to change the javadocs, I have create  a ccc request which I will do 
as part of the JDBC 4.2 work and will put the change back at that time.

If you would like to change your test and then create similar tests for 
SerialClob I will add those as part of the put-back.

Again, thank you for your input and time.

Best
Lance




On Jul 5, 2012, at 2:26 AM, Deven You wrote:

 Hi Lance,
 
 Did you review the patch and compare it to yours. I just have some more words 
 for the patch.
 
 1. I think the current spec for free() is not clear, how about add below 
 comments:
 
 After free has been called, any attempt to invoke a method other than 
 free will result in a  SerialException being thrown.
 
 2. getBinaryStream(long pos,long length)
 
 add a javadoc:
 * @throws SerialException if this SerialBlob already be freed.
 
 add throws SerialException from this method
 
 Any suggestions?
 
 Thanks a lot!
 On 07/02/2012 06:25 PM, Lance Andersen - Oracle wrote:
 Hi Deven,
 
 Thanks for the email and the proposed patch.  I will look at this later 
 today or tomorrow.  I actually have made these changes in my workspace for 
 JDK 8 but will compare your changes to mine.
 
 Best
 Lance
 On Jul 2, 2012, at 5:04 AM, Deven You wrote:
 
 Hi All,
 Could anyone notice this problem?
 
 Thanks a lot!
 On 06/25/2012 04:18 PM, Deven You wrote:
 Hi All,
 
 First of all, if the jdbc problem has a better mailing list to post please 
 tell me.
 
 I find that javax.sql.rowset.serial.SerialBlob is not fully implemented in 
 OpenJDK 8. Methods
 
public InputStream getBinaryStream(long pos,long length) throws 
 SQLException
public void free() throws SQLException
 
 only throw UnsupportedOperationException.
 
 I have made a patch[1] to implement these 2 methods. Could anyone take a 
 look to review it.
 
 BTW: I think the spec for SerialBlob is not very clear like it doesn't 
 mention if all method rather than free() need throw any exception after 
 free() is invoked. However that behavior seems reasonable.
 
 [1] http://cr.openjdk.java.net/~littlee/OJDK-576/webrev
 
 Thanks a lot.
 
 
 
 -- 
 Best Regards,
 
 Deven
 
 
 
 Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
 Oracle Java Engineering 
 1 Network Drive 
 Burlington, MA 01803
 lance.ander...@oracle.com
 
 
 
 -- 
 Best Regards,
 
 Deven


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: RFR [7u8]: 7166896: DocumentBuilder.parse(String uri) is not IPv6 enabled. It throws MalformedURLException

2012-07-12 Thread Lance Andersen - Oracle
+1

Best
Lance
On Jul 12, 2012, at 5:42 AM, Paul Sandoz wrote:

 Hi Joe,
 
 On Jul 11, 2012, at 7:59 AM, Joe Wang wrote:
 
 Hi Paul,
 
 This is now for 7u8, so I started a new thread.
 
 As we discussed, I've removed the hack of using escapeNonUSAscii.  In this 
 regard, we're now in sync with the source code in Xerces.
 
 Here's the webrev:
 http://cr.openjdk.java.net/~joehw/7u8/7166896/webrev/
 
 
 Looks OK to me. Note i am not an official reviewer, yet! so please don't 
 commit cause i thought it looked OK ;-)
 
 Paul.


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Review request 7190657 Modify getDriver() to call Thread.currentThread().getContextClassLoader();

2012-08-10 Thread Lance Andersen - Oracle
Looking for a reviewer for the following change:

-  add  a call to Thread.currentThread().getContextClassLoader() to 
DriverManager.getDriver()
- Remove the synchronized block for the same call in getConnection()

Thank you.

Best
Lance

localhost:sql lanceandersen$ hg diff DriverManager.java
diff -r 629f357fc17b src/share/classes/java/sql/DriverManager.java
--- a/src/share/classes/java/sql/DriverManager.java Fri Aug 10 09:17:14 
2012 -0400
+++ b/src/share/classes/java/sql/DriverManager.java Fri Aug 10 13:05:00 
2012 -0400
@@ -264,6 +264,10 @@
 // be null.
 ClassLoader callerCL = DriverManager.getCallerClassLoader();
 
+if(callerCL == null) {
+callerCL = Thread.currentThread().getContextClassLoader();
+}
+
 // Walk through the loaded registeredDrivers attempting to locate 
someone
 // who understands the given URL.
 for (DriverInfo aDriver : registeredDrivers) {
@@ -563,11 +567,8 @@
  * classloader, so that the JDBC driver class outside rt.jar
  * can be loaded from here.
  */
-synchronized(DriverManager.class) {
-  // synchronize loading of the correct classloader.
-  if(callerCL == null) {
-  callerCL = Thread.currentThread().getContextClassLoader();
-   }
+if(callerCL == null) {
+callerCL = Thread.currentThread().getContextClassLoader();
 }
 
 if(url == null) {
localhost:sql lanceandersen$ 

Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: Review request 7190657 Modify getDriver() to call Thread.currentThread().getContextClassLoader();

2012-08-11 Thread Lance Andersen - Oracle
For completeness, they could be added and I guess I can do it as part of this 
particular change.

I would probably move the call to  DriverManager.getCallerClassLoader() to 
avoid the duplication of the if block.

Best
Lance
On Aug 10, 2012, at 5:02 PM, David Schlosnagle wrote:

 Lance,
 
 There are a couple of other uses of
 DriverManager.getCallerClassLoader() that do no fall back to
 Thread.currentThread().getContextClassLoader() if the caller class
 loader is null (bootstrap loader). Should these be updated as well?
 
 From 
 http://hg.openjdk.java.net/jdk8/jdk8/jdk/file/tip/src/share/classes/java/sql/DriverManager.java:
  Line 334 in deregisterDriver(Driver driver)
  Line 365 in getDrivers()
 
 Thanks,
 Dave
 
 
 On Fri, Aug 10, 2012 at 1:19 PM, Lance Andersen - Oracle
 lance.ander...@oracle.com wrote:
 Looking for a reviewer for the following change:
 
 -  add  a call to Thread.currentThread().getContextClassLoader() to 
 DriverManager.getDriver()
 - Remove the synchronized block for the same call in getConnection()
 
 Thank you.
 
 Best
 Lance
 
 localhost:sql lanceandersen$ hg diff DriverManager.java
 diff -r 629f357fc17b src/share/classes/java/sql/DriverManager.java
 --- a/src/share/classes/java/sql/DriverManager.java Fri Aug 10 09:17:14 
 2012 -0400
 +++ b/src/share/classes/java/sql/DriverManager.java Fri Aug 10 13:05:00 
 2012 -0400
 @@ -264,6 +264,10 @@
 // be null.
 ClassLoader callerCL = DriverManager.getCallerClassLoader();
 
 +if(callerCL == null) {
 +callerCL = Thread.currentThread().getContextClassLoader();
 +}
 +
 // Walk through the loaded registeredDrivers attempting to locate 
 someone
 // who understands the given URL.
 for (DriverInfo aDriver : registeredDrivers) {
 @@ -563,11 +567,8 @@
  * classloader, so that the JDBC driver class outside rt.jar
  * can be loaded from here.
  */
 -synchronized(DriverManager.class) {
 -  // synchronize loading of the correct classloader.
 -  if(callerCL == null) {
 -  callerCL = Thread.currentThread().getContextClassLoader();
 -   }
 +if(callerCL == null) {
 +callerCL = Thread.currentThread().getContextClassLoader();
 }
 
 if(url == null) {
 localhost:sql lanceandersen$
 
 Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
 Oracle Java Engineering
 1 Network Drive
 Burlington, MA 01803
 lance.ander...@oracle.com
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: RFR: 7056731: Race condition in CORBA code causes re-use of ABORTed connections

2012-08-15 Thread Lance Andersen - Oracle
Hi Sean,

This looks good.

Best
Lance
On Aug 14, 2012, at 12:38 PM, Seán Coffey wrote:

 I'm looking to forward port this corba fix from 6u34 to jdk8 (and eventually 
 port to 7u)
 
 d.macdon...@auckland.ac.nz originally reported this issue and I'll be marking 
 the fix as contributed by him. He's already signed the OCA. There's a good 
 description in bug report. We have a race like condition if the corba server 
 is shut down while there are still workers/client threads in a waiting queue 
 waiting for server. The issue arises when the purgeCalls method marks a 
 socket state as ABORTed or CLOSE_RECVD but fails to remove that socket from 
 the socket connectionCache. Fix is to wrap the removal of the socket in a 
 finally block.
 
 The CorbaResponseWaitingRoomImpl changes are more focused on ensuring that 
 the Map holding these clients is better synchronized.
 
 webrev : http://cr.openjdk.java.net/~coffeys/webrev.7056731.jdk8/ 
 http://cr.openjdk.java.net/%7Ecoffeys/webrev.7056731.jdk8/
 bug report : http://bugs.sun.com/view_bug.do?bug_id=7056731
 
 Regards,
 Sean.


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: RFR [7u8] (JAXP): 7191547 : XMLEventFactory.newFactory(String factoryId, ClassLoader loader) does not work as expected

2012-08-17 Thread Lance Andersen - Oracle
Hi Joe,

Looks fine.

Best
Lance
On Aug 17, 2012, at 12:07 AM, Joe Wang wrote:

 In the patch for 6756677, we fixed errors in StAX's input and output 
 factories where factoryId such as 'javax.xml.stream.XMLInputFactory was 
 taken as factory class e.g. com.sun.xml.internal.stream.XMLInputFactoryImpl.  
 Unfortunately, we missed XMLEventFactory in that patch.
 
 So this is a patch for XMLEventFactory similar to that of XMLInputFactory and 
 XMLOutputFactory.  The change itself is to search an implementation class 
 using the provided factory id instead of trying to create a instance directly.
 
 The webrev is here:  http://cr.openjdk.java.net/~joehw/7u8/7191547/webrev/
 
 All tests pass except two XMLEventFactory tests that assumed the factoryId to 
 mean factory class. They will be updated once this patch is approved.
 
 Thanks,
 Joe
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



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 Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Review request for 7192302 Remove JDBCRowSetImpl dependency on java.beans

2012-09-05 Thread Lance Andersen - Oracle
Hi all,

Looking for a reviewer for the removal of PropertyChangeSupport from 
JDBCRowSetImpl that was originally going to be used by the EOL Rave product.  
As it is no longer needed the code has been removed.  The SQE and RowSet TCK 
tests all continue to run without regression.

The webrev can be found at http://cr.openjdk.java.net/~lancea/7192302/webrev.00

Best
Lance

Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: Review request for 7192302 Remove JDBCRowSetImpl dependency on java.beans

2012-09-06 Thread Lance Andersen - Oracle
Here is the updated webrev http://cr.openjdk.java.net/~lancea/7192302/webrev.01

I know there is more clean-up that can be done to remove other Rave added code 
(such as the removal of set/getPreparedStatement/Connection/ResultSet), I want 
to keep the focus to just removing PropertyChangeSupport.  SQE and RowSet TCKs 
continue to pass with these changes.

Best
Lance
On Sep 5, 2012, at 5:17 PM, Alan Bateman wrote:

 On 05/09/2012 22:04, Lance Andersen - Oracle wrote:
 Hi all,
 
 Looking for a reviewer for the removal of PropertyChangeSupport from 
 JDBCRowSetImpl that was originally going to be used by the EOL Rave product. 
  As it is no longer needed the code has been removed.  The SQE and RowSet 
 TCK tests all continue to run without regression.
 
 The webrev can be found at 
 http://cr.openjdk.java.net/~lancea/7192302/webrev.00
 
 Thanks Lance, it's good to remove this dependency. In both commit and 
 rollback then it looks to me that the setting of oldVal can be removed. 
 Otherwise looks good to me.
 
 -Alan


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: Review request for 7192302 Remove JDBCRowSetImpl dependency on java.beans

2012-09-06 Thread Lance Andersen - Oracle
Thank you for the comments Alan

On Sep 6, 2012, at 9:00 AM, Alan Bateman wrote:

 On 06/09/2012 13:40, Lance Andersen - Oracle wrote:
 Here is the updated webrev 
 http://cr.openjdk.java.net/~lancea/7192302/webrev.01
 
 I know there is more clean-up that can be done to remove other Rave added 
 code (such as the removal of set/getPreparedStatement/Connection/ResultSet), 
 I want to keep the focus to just removing PropertyChangeSupport.  SQE and 
 RowSet TCKs continue to pass with these changes.
 
 Your previous mail uses the word nuke, I was thinking the same thing :-)
 
 The latest webrev looks okay except that in one of the constructors you have 
 removed a call to ensure that the connection is established, I'm not sure 
 about the significance of that.

This is not needed here and given I have already tested with this removed, I 
figured I would OK to keep this as part of the change
 
 I also see there are a couple of residual references to Rave that can 
 probably be pulled, these seem to be related to the PropertyChangeListener 
 support.

I left those in as a reminder to go back as part of the rest of the Rave 
clean-up.  I would prefer to leave them for now and when I  make another pass 
for Rave, I will get rid of them
 
 One method that looks like it could be removed too is setConcurrency but I 
 agree that keeping focused on just removing the beans dependency is right for 
 now.

 I had thought about that but I have to think about this one a bit more as the 
getConcurrency() is leveraging the value returned from the active ResultSet 
which is why I did not remove this at this time.

In some cases, I must confess the original authors have left me scratching my 
head on some of this code but I want to be surgical in removing or addressing 
changes so that it would be easier to find any un-intended gotchas.

Best
Lance

 
 -Alan.


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: Review request for 7192302 Remove JDBCRowSetImpl dependency on java.beans

2012-09-06 Thread Lance Andersen - Oracle

On Sep 6, 2012, at 9:31 AM, Alan Bateman wrote:

 On 06/09/2012 14:09, Lance Andersen - Oracle wrote:
 :
 
 The latest webrev looks okay except that in one of the constructors you 
 have removed a call to ensure that the connection is established, I'm not 
 sure about the significance of that.
 
 This is not needed here and given I have already tested with this removed, I 
 figured I would OK to keep this as part of the change
 Okay, I'll have to trust you on this one but I am a little bit concerned that 
 it could cause a NPE, say someone creates a JdbcRowSet and invokes a method 
 such as comment, rollback or getAutoCommit without doing an explicit connect.

connect() is typically called to get a connection and it will validate that 
there is a non-null Connection.  I have unit tests which leverage that 
constructor and run clean with that change.

I agree that commit(), rollback, etc  should call checkState() and that is 
another fix I need to do but the potential for the NPE is there depending on 
which constructor was used and if you did something silly like making a 
rollback without doing any work.

I can revert that change if you like but I have not seen any failures under 
what I would term normal use.
 
 
 
 I left those in as a reminder to go back as part of the rest of the Rave 
 clean-up.  I would prefer to leave them for now and when I  make another 
 pass for Rave, I will get rid of them
 Okay.
 
 
 One method that looks like it could be removed too is setConcurrency but I 
 agree that keeping focused on just removing the beans dependency is right 
 for now.
 
 I had thought about that but I have to think about this one a bit more as 
 the getConcurrency() is leveraging the value returned from the active 
 ResultSet which is why I did not remove this at this time.
 
 Okay although it looks to be just an optimization introduced as part of 
 adding listener events

I am not sure I agree with the code either, just want to spend a bit more time 
looking at the code flow and see what tests if any we have here
  I'm fine with leaving it as it is.

thank you
 
 -Alan


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: Review request for 7192302 Remove JDBCRowSetImpl dependency on java.beans

2012-09-06 Thread Lance Andersen - Oracle
Hi Mandy,

Thank you also for the feedback along with Alan. I will push these back shortly 
and also create a CR.

Best
Lance
On Sep 6, 2012, at 11:04 AM, Mandy Chung wrote:

 Lance,
 
 On 9/6/2012 5:40 AM, Lance Andersen - Oracle wrote:
 Here is the updated webrev 
 http://cr.openjdk.java.net/~lancea/7192302/webrev.01
 
 It's good to see this change that jdbc rowset doesn't depend on java.beans.
 
 This looks okay to me as you have explained why you remove the call to 
 connect() in one of the constructors.  Alan and you discussed if 
 setConcurrency could be removed in this fix. setType() is another one that 
 could be removed.   I'm fine with leaving these cleanup in another fix and 
 have this fix to focus on removing the dependency of PropertyChangeSupport.  
 It'd be good to file a CR as a follow-up of this work.
 
 Thanks
 Mandy
 
 I know there is more clean-up that can be done to remove other Rave added 
 code (such as the removal of set/getPreparedStatement/Connection/ResultSet), 
 I want to keep the focus to just removing PropertyChangeSupport.  SQE and 
 RowSet TCKs continue to pass with these changes.
 
 Best
 Lance
 On Sep 5, 2012, at 5:17 PM, Alan Bateman wrote:
 
 On 05/09/2012 22:04, Lance Andersen - Oracle wrote:
 Hi all,
 
 Looking for a reviewer for the removal of PropertyChangeSupport from 
 JDBCRowSetImpl that was originally going to be used by the EOL Rave 
 product.  As it is no longer needed the code has been removed.  The SQE 
 and RowSet TCK tests all continue to run without regression.
 
 The webrev can be found at 
 http://cr.openjdk.java.net/~lancea/7192302/webrev.00
 
 Thanks Lance, it's good to remove this dependency. In both commit and 
 rollback then it looks to me that the setting of oldVal can be removed. 
 Otherwise looks good to me.
 
 -Alan
 
 Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
 Oracle Java Engineering
 1 Network Drive
 Burlington, MA 01803
 lance.ander...@oracle.com
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: Review request for 7192302 Remove JDBCRowSetImpl dependency on java.beans

2012-09-06 Thread Lance Andersen - Oracle
Hi Alan,

The connect method is used by the RI not the RowSet spec.  It was made 
protected for Rave.

Best
Lance
On Sep 6, 2012, at 1:21 PM, Alan Bateman wrote:

 Lance,
 
 I see you've just pushed this but one thing I didn't spot initially is that 
 in the second webrev you changed the protected connect method to be private. 
 Is this a supported API and could this cause problems for JDBC drivers that 
 extend this?
 
 -Alan


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: Review request for 7192302 Remove JDBCRowSetImpl dependency on java.beans

2012-09-06 Thread Lance Andersen - Oracle
The recommended way to access a rowset is via the factory now.For whatever 
reason the original authors chose not to provide a factory.

The connect() method has always been a method to be used internally by 
JdbcRowSetRIImpl and Rave.  There is no reason for a user of the API to ever 
have to call this method.   I can certainly update the javadocs for JdbcRowSet 
to use the factory but all of the examples as well as the tutorial always 
leveraged using the defined API.

Best
Lance
On Sep 6, 2012, at 1:54 PM, Mandy Chung wrote:

 As the comment noted, the connect method was made protected for Rave 
 requirement but it's not defined in JdbcRowSet and BaseRowSet.  It's a method 
 specific to the RI and changing it to private is okay if they get an 
 JdbcRowSet instance by calling the rowset factory.  Alan's question prompts 
 me to look if anyone references JdbcRowSetImpl directly - I find that the 
 javax.sql.rowset.JdbcRowSet spec has the example to instantiate 
 JdbcRowSetImpl.  Is it still a supported or recommended way?  If so, there is 
 a potential compatibility risk; otherwise, it'd be good to update the spec to 
 remove reference to JdbcRowSetImpl.
 
 Mandy
 
 On 9/6/2012 10:21 AM, Alan Bateman wrote:
 Lance,
 
 I see you've just pushed this but one thing I didn't spot initially is that 
 in the second webrev you changed the protected connect method to be private. 
 Is this a supported API and could this cause problems for JDBC drivers that 
 extend this?
 
 -Alan


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: Review Request for 7195933: There is incorrect link to Info-ZIP Application Note 970311 in doc page of Package java.util.zip

2012-09-18 Thread Lance Andersen - Oracle
looks OK Dan and I was able to access the URL and download the zip

Best
Lance
On Sep 18, 2012, at 1:29 AM, Dan Xu wrote:

 Hi,
 
 This is the change to correct a java doc link in java.util.zip package page.
 
 The webrev can be reviewed at 
 http://cr.openjdk.java.net/~dxu/7195933/webrev/. Thanks!
 
 -Dan


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Review request for 7197395

2012-09-29 Thread Lance Andersen - Oracle
Hi,

This is a review request for adding @Deprecated to the remaining JDBC methods 
to suppress compiler warnings.  The CCC was approved as this annotation results 
in the signatures changing (though they do not effect the execution of any 
applications).

The webrev is http://cr.openjdk.java.net/~lancea/7197395/webrev.00/

Best
Lance

Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: JAXP RFR: 8000172 : 2 SAX features does not work properly

2012-10-09 Thread Lance Andersen - Oracle
Look OK Joe


On Oct 9, 2012, at 12:31 AM, Joe Wang wrote:

 This is an issue found while I was working with SQE to expand test coverage. 
 For a non-validating parser, when load-external-dtd is false, entity 
 references are skipped. However, the skippedEntity() event was not reported.
 
 The problem was due to missing infoset augmentations when document handler's 
 startGeneralEntity/endGeneralEntity were called.  The fix therefore is simply 
 add the arguments.
 
 webrev: http://cr.openjdk.java.net/~joehw/jdk8/8000172/webrev/
 
 Please review.
 
 Thanks,
 Joe


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: RFR : 7196086 : update copyright years for files in corba repository (JDK 8)

2012-10-09 Thread Lance Andersen - Oracle
looks fine Sean

Best
Lance
On Oct 9, 2012, at 2:38 PM, Seán Coffey wrote:

 steve.si...@oracle.com has contributed the following patch which I'd like to 
 push to jdk8 TL. It's the correction of copyright years in corba repo.
 Need a reviewer.
 
 webrev :
 http://cr.openjdk.java.net/~coffeys/webrev.7196086.jdk8/ 
 http://cr.openjdk.java.net/%7Ecoffeys/webrev.7196086.jdk8/
 
 regards,
 Sean.
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: 7192274: Deprecate LogManager addPropertyChangeListener and removePropertyChangeListe

2012-10-10 Thread Lance Andersen - Oracle
looks fine Alan and in line with the other work we have done

Best
Lance
On Oct 10, 2012, at 7:19 AM, Alan Bateman wrote:

 
 JEP 162 [1] captures a number of things that we can do in preparation for 
 future modularization of the platform. One of these items is deprecating the 
 Java SE APIs that are problematic for our modularization efforts. Thankfully 
 the list is very short as this is deprecation is in anticipation of really 
 removing the APIs when a module system comes along.
 
 The patch proposed here is the first installation to deprecate the LogManager 
 addPropertyChangeListener and removePropertyChangeListener methods. These 
 methods are problematic because of the API dependency on 
 java.beans.PropertyChangeListener (java.beans is toxic because of types in 
 that package that are tied to Applet, AWT, and Swing). When we eventually get 
 to remove these methods then we expect the impact will not be too 
 significant. Mandy Chung has done a static analysis over 20,000 projects to 
 locate usages of these methods and found only 3 so we have some useful 
 evidence to demonstrate that they aren't used very often. For now we are just 
 proposing to deprecate the methods and the proposed patch is here:
 
 http://cr.openjdk.java.net/~alanb/7192274/webrev/jdk.patch
 
 Thanks,
 Alan.
 
 [1] http://openjdk.java.net/jeps/162


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Review needed for 8000687

2012-10-10 Thread Lance Andersen - Oracle
Need a reviewer for a simple typo in the DriverManager javadoc

new-host-2:sql lanceandersen$ hg diff
diff -r 036c55976cef src/share/classes/java/sql/DriverManager.java
--- a/src/share/classes/java/sql/DriverManager.java Tue Oct 09 08:58:27 
2012 -0400
+++ b/src/share/classes/java/sql/DriverManager.java Wed Oct 10 10:59:00 
2012 -0400
@@ -412,7 +412,7 @@
  * method throws a codejava.lang.SecurityException/code.
  *
  * @param out the new logging/tracing PrintStream; to disable, set to 
codenull/code
- * @deprecated Use {@code setLogWriter)
+ * @deprecated Use {@code setLogWriter}
  * @throws SecurityException if a security manager exists and its
  *codecheckPermission/code method denies setting the log stream
  *
@@ -439,7 +439,7 @@
  * and all drivers.
  *
  * @return the logging/tracing PrintStream; if disabled, is 
codenull/code
- * @deprecated  Use {@code getLogWriter)
+ * @deprecated  Use {@code getLogWriter}
  * @see #setLogStream
  */
 @Deprecated
new-host-2:sql lanceandersen$ 


Best
Lance

Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Review Request for 8000712

2012-10-10 Thread Lance Andersen - Oracle
Hi,  

Looking for a reviewer for the removal of the following non-used fields in 
SyncFactory

 private static String default_provider
private static Level rsLevel
private static Object logSync
private static java.io.PrintWriter logWriter


Best
Lance

new-host-2:spi lanceandersen$ hg diff SyncFactory.java 
diff -r 3c4be36de073 src/share/classes/javax/sql/rowset/spi/SyncFactory.java
--- a/src/share/classes/javax/sql/rowset/spi/SyncFactory.java   Wed Oct 10 
11:15:27 2012 -0400
+++ b/src/share/classes/javax/sql/rowset/spi/SyncFactory.java   Wed Oct 10 
16:57:46 2012 -0400
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2011, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2012, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -229,11 +229,7 @@
  * The standard resource file name.
  */
 private static String ROWSET_PROPERTIES = rowset.properties;
-/**
- * The RI Optimistic Provider.
- */
-private static String default_provider =
-com.sun.rowset.providers.RIOptimisticProvider;
+
 /**
  *  Permission required to invoke setJNDIContext and setLogger
  */
@@ -248,24 +244,13 @@
  * The codeLogger/code object to be used by the 
codeSyncFactory/code.
  */
 private static volatile Logger rsLogger;
-/**
- *
- */
-private static Level rsLevel;
+
 /**
  * The registry of available codeSyncProvider/code implementations.
  * See section 2.0 of the class comment for codeSyncFactory/code for an
  * explanation of how a provider can be added to this registry.
  */
 private static HashtableString, SyncProvider implementations;
-/**
- * Internal sync object used to maintain the SPI as a singleton
- */
-private static Object logSync = new Object();
-/**
- * Internal PrintWriter field for logging facility
- */
-private static java.io.PrintWriter logWriter = null;
 
 /**
  * Adds the the given synchronization provider to the factory register. 
Guidelines
new-host-2:spi lanceandersen$




Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: 8000362: (pack200) Deprecate Packer/Unpacker addPropertyChangeLister and removePropertyChangeListener methods

2012-10-11 Thread Lance Andersen - Oracle
looks fine alan
On Oct 11, 2012, at 9:40 AM, Alan Bateman wrote:

 
 This is a follow-on from yesterday's mail on deprecating the LogManager's 
 add/removePropertyChangeListener methods. The other 4 problematic methods 
 identified in JEP 162 [1] are the same name methods on Pack200.Packer and 
 Pack200.Unpacker. The webrev to deprecate these methods is here:
 
 http://cr.openjdk.java.net/~alanb/8000362/webrev/
 
 As with the LogManager methods, Mandy Chung searched 2 projects. In this 
 case we didn't find any usages of these methods and this gives us confidence 
 that these methods are rarely used. We don't propose to provide a replacement 
 for these methods but instead just include wording to suggest that 
 applications poll the value of the progress property if they really need to 
 provide some progress indication.
 
 Kumar is the pack200 guru here and I know he's busy/away at the moment so I 
 plan to hold onto this patch until I at least hear from him.
 
 -Alan.
 
 [1] http://openjdk.java.net/jeps/162


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Reviewer needed for 8000763: use XXX.valueOf methods instead of constructors

2012-10-11 Thread Lance Andersen - Oracle
Hi,

Need a review for changing to use the XXX.valueOf methods from constructors.

Thank you

Best
Lance



new-host-2:rowset lanceandersen$ hg status -m
M src/share/classes/com/sun/rowset/CachedRowSetImpl.java
M src/share/classes/com/sun/rowset/FilteredRowSetImpl.java
M src/share/classes/javax/sql/rowset/BaseRowSet.java
M src/share/classes/javax/sql/rowset/serial/SQLOutputImpl.java

new-host-2:rowset lanceandersen$ hg diff
diff -r c2be39b27e1c src/share/classes/com/sun/rowset/CachedRowSetImpl.java
--- a/src/share/classes/com/sun/rowset/CachedRowSetImpl.javaThu Oct 11 
11:47:05 2012 +0100
+++ b/src/share/classes/com/sun/rowset/CachedRowSetImpl.javaThu Oct 11 
12:43:57 2012 -0400
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2011, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2012, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -1747,7 +1747,7 @@
 // convert to a Double and compare to zero
 try {
 Double d = new Double(value.toString());
-if (d.compareTo(new Double((double)0)) == 0) {
+if (d.compareTo(Double.valueOf(0)) == 0) {
 return false;
 } else {
 return true;
@@ -4432,7 +4432,7 @@
 // make sure the cursor is on a valid row
 checkCursor();
 
-Object obj = convertNumeric(new Float(x),
+Object obj = convertNumeric(Float.valueOf(x),
 java.sql.Types.REAL,
 RowSetMD.getColumnType(columnIndex));
 
@@ -4467,7 +4467,7 @@
 checkIndex(columnIndex);
 // make sure the cursor is on a valid row
 checkCursor();
-Object obj = convertNumeric(new Double(x),
+Object obj = convertNumeric(Double.valueOf(x),
 java.sql.Types.DOUBLE,
 RowSetMD.getColumnType(columnIndex));
 
diff -r c2be39b27e1c src/share/classes/com/sun/rowset/FilteredRowSetImpl.java
--- a/src/share/classes/com/sun/rowset/FilteredRowSetImpl.java  Thu Oct 11 
11:47:05 2012 +0100
+++ b/src/share/classes/com/sun/rowset/FilteredRowSetImpl.java  Thu Oct 11 
12:43:57 2012 -0400
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2010, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2012, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -839,7 +839,7 @@
 
   if(onInsertRow) {
  if(p != null) {
-bool = p.evaluate(new Float(x) , columnIndex);
+bool = p.evaluate(Float.valueOf(x) , columnIndex);
 
 if(!bool) {
throw new 
SQLException(resBundle.handleGetObject(filteredrowsetimpl.notallowed).toString());
@@ -906,7 +906,7 @@
 
   if(onInsertRow) {
  if(p != null) {
-bool = p.evaluate(new Double(x) , columnIndex);
+bool = p.evaluate(Double.valueOf(x) , columnIndex);
 
 if(!bool) {
throw new 
SQLException(resBundle.handleGetObject(filteredrowsetimpl.notallowed).toString());
diff -r c2be39b27e1c src/share/classes/javax/sql/rowset/BaseRowSet.java
--- a/src/share/classes/javax/sql/rowset/BaseRowSet.javaThu Oct 11 
11:47:05 2012 +0100
+++ b/src/share/classes/javax/sql/rowset/BaseRowSet.javaThu Oct 11 
12:43:57 2012 -0400
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2011, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2012, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -1850,7 +1850,7 @@
 if(params == null){
  throw new SQLException(Set initParams() before setFloat);
 }
-params.put(Integer.valueOf(parameterIndex - 1), new Float(x));
+params.put(Integer.valueOf(parameterIndex - 1), Float.valueOf(x));
 }
 
 /**
@@ -1882,7 +1882,7 @@
 if(params == null){
  throw new SQLException(Set initParams() before setDouble);
 }
-params.put(Integer.valueOf(parameterIndex - 1), new Double(x));
+params.put(Integer.valueOf(parameterIndex - 1), Double.valueOf(x));
 }
 
 /**
diff -r c2be39b27e1c 
src/share/classes/javax/sql/rowset/serial/SQLOutputImpl.java
--- a/src/share/classes/javax/sql/rowset/serial/SQLOutputImpl.java  Thu Oct 
11 11:47:05 2012 +0100
+++ b/src/share/classes/javax/sql/rowset/serial/SQLOutputImpl.java  Thu Oct 
11 12:43:57 2012 -0400
@@ -215,7 +215,7 @@
  */
 @SuppressWarnings(unchecked)
 public void writeFloat(float x) throws SQLException {
-attribs.add(new Float(x));
+attribs.add(Float.valueOf(x));
 }
 
 /**
@@ -230,7 +230,7 @@
  */
 @SuppressWarnings(unchecked)
 

Resend: Reviewer needed for 8000763: use XXX.valueOf methods instead of constructors

2012-10-11 Thread Lance Andersen - Oracle
Hi,

Revised CachedRowSetImpl  as I noticed Findbugs missed a scenario where  you 
should use the XXX.valueOf methods from constructors.

Thank you

Best
Lance



new-host-2:rowset lanceandersen$ hg status -m
M src/share/classes/com/sun/rowset/CachedRowSetImpl.java
M src/share/classes/com/sun/rowset/FilteredRowSetImpl.java
M src/share/classes/javax/sql/rowset/BaseRowSet.java
M src/share/classes/javax/sql/rowset/serial/SQLOutputImpl.java

new-host-2:rowset lanceandersen$ hg diff
diff -r c2be39b27e1c src/share/classes/com/sun/rowset/CachedRowSetImpl.java
--- a/src/share/classes/com/sun/rowset/CachedRowSetImpl.javaThu Oct 11 
11:47:05 2012 +0100
+++ b/src/share/classes/com/sun/rowset/CachedRowSetImpl.javaThu Oct 11 
13:17:26 2012 -0400
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2011, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2012, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -1746,8 +1746,8 @@
 
 // convert to a Double and compare to zero
 try {
-Double d = new Double(value.toString());
-if (d.compareTo(new Double((double)0)) == 0) {
+Double d = Double.valueOf(value.toString());
+if (d.compareTo(Double.valueOf(0)) == 0) {
 return false;
 } else {
 return true;
@@ -4432,7 +4432,7 @@
 // make sure the cursor is on a valid row
 checkCursor();
 
-Object obj = convertNumeric(new Float(x),
+Object obj = convertNumeric(Float.valueOf(x),
 java.sql.Types.REAL,
 RowSetMD.getColumnType(columnIndex));
 
@@ -4467,7 +4467,7 @@
 checkIndex(columnIndex);
 // make sure the cursor is on a valid row
 checkCursor();
-Object obj = convertNumeric(new Double(x),
+Object obj = convertNumeric(Double.valueOf(x),
 java.sql.Types.DOUBLE,
 RowSetMD.getColumnType(columnIndex));
 
diff -r c2be39b27e1c src/share/classes/com/sun/rowset/FilteredRowSetImpl.java
--- a/src/share/classes/com/sun/rowset/FilteredRowSetImpl.java  Thu Oct 11 
11:47:05 2012 +0100
+++ b/src/share/classes/com/sun/rowset/FilteredRowSetImpl.java  Thu Oct 11 
13:17:26 2012 -0400
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2010, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2012, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -839,7 +839,7 @@
 
   if(onInsertRow) {
  if(p != null) {
-bool = p.evaluate(new Float(x) , columnIndex);
+bool = p.evaluate(Float.valueOf(x) , columnIndex);
 
 if(!bool) {
throw new 
SQLException(resBundle.handleGetObject(filteredrowsetimpl.notallowed).toString());
@@ -906,7 +906,7 @@
 
   if(onInsertRow) {
  if(p != null) {
-bool = p.evaluate(new Double(x) , columnIndex);
+bool = p.evaluate(Double.valueOf(x) , columnIndex);
 
 if(!bool) {
throw new 
SQLException(resBundle.handleGetObject(filteredrowsetimpl.notallowed).toString());
diff -r c2be39b27e1c src/share/classes/javax/sql/rowset/BaseRowSet.java
--- a/src/share/classes/javax/sql/rowset/BaseRowSet.javaThu Oct 11 
11:47:05 2012 +0100
+++ b/src/share/classes/javax/sql/rowset/BaseRowSet.javaThu Oct 11 
13:17:26 2012 -0400
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2011, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2012, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -1850,7 +1850,7 @@
 if(params == null){
  throw new SQLException(Set initParams() before setFloat);
 }
-params.put(Integer.valueOf(parameterIndex - 1), new Float(x));
+params.put(Integer.valueOf(parameterIndex - 1), Float.valueOf(x));
 }
 
 /**
@@ -1882,7 +1882,7 @@
 if(params == null){
  throw new SQLException(Set initParams() before setDouble);
 }
-params.put(Integer.valueOf(parameterIndex - 1), new Double(x));
+params.put(Integer.valueOf(parameterIndex - 1), Double.valueOf(x));
 }
 
 /**
diff -r c2be39b27e1c 
src/share/classes/javax/sql/rowset/serial/SQLOutputImpl.java
--- a/src/share/classes/javax/sql/rowset/serial/SQLOutputImpl.java  Thu Oct 
11 11:47:05 2012 +0100
+++ b/src/share/classes/javax/sql/rowset/serial/SQLOutputImpl.java  Thu Oct 
11 13:17:26 2012 -0400
@@ -215,7 +215,7 @@
  */
 @SuppressWarnings(unchecked)
 public void writeFloat(float x) throws SQLException {
-attribs.add(new Float(x));
+

Re: Review Request: JDK-8001565, (fs) Typo Path.endsWith(String) javadoc

2012-10-25 Thread Lance Andersen - Oracle
This fix is fine Dan

Best
Lance
On Oct 25, 2012, at 5:45 PM, Dan Xu wrote:

 Hi,
 
 Please help review the javadoc typo fix at, 
 http://cr.openjdk.java.net/~dxu/8001565/webrev/. Thanks!
 
 -Dan
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: javax.sql.rowset.serial.SerialBlob doesn't support free and getBinaryStream

2012-10-29 Thread Lance Andersen - Oracle
Hi Deven,

I will address the needed updates a bit later.

Thank you for your input

Best
Lance
On Oct 29, 2012, at 3:51 AM, Deven You wrote:

 Hi Alan,
 
 The Java Spec does not mention the thread safe for JDBC API. But I see the 
 other code in SerialBlob/SerialClob have not consider it.
 
 I think use buff == null to replace isFree is a good idea because it also 
 avoid the problem for the condition buf == null  isFree == false so we 
 won't need create a readObject method.
 
 Thanks for your suggestion for isFree, I will correct it later.
 
 Lance: How about your suggestion? Since you mentioned you will develop the 
 implementation yourself. I use my implementation mainly for the test cases. 
 But you may also take a look my implementation.
 
 Thanks a lot!
 
 On 09/21/2012 04:37 PM, Alan Bateman wrote:
 On 21/09/2012 04:21, Deven You wrote:
 Hi Lance,
 
 I am very busy with other work so I can't work with the 
 SerialBlob/SerialClob item for long time. I am very happy to refine the 
 current test case and create new tests for SerialClob.
 
 I have create a new webre[1] for this task, please review it.
 
 [1] http://cr.openjdk.java.net/~youdwei/OJDK-576/webrev.01/ 
 http://cr.openjdk.java.net/%7Eyoudwei/OJDK-576/webrev.01/
 
 PS: If the isFree is not transient, I want to know how we add this field to 
 the javadoc serialized form?
 I don't know very much about the rowset API and I can't see anything to 
 specify whether it is meant to be safe for use by concurrent threads. There 
 are clearly lots of issues here and implementing free introduces a lot more, 
 especially with the possibility of an asynchronous free or more than one 
 thread calling free at around the same time.
 
 Have you considered buf == null to mean that the resources are freed? That 
 might avoid needing to change the serialized form. Also as these types are 
 serializable it means you have to consider the case where you deserialize to 
 buf == null  isFree == false for example. On that point, it looks to me 
 that this code needs a readObject anyway (for several reasons).
 
 A small point is that isFree is a odd name for a method that doesn't 
 return a boolean. If the patch goes ahead then I think it needs a better 
 name, ensureNotFree or requireNotFree or something like that.
 
 -Alan.
 
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Review request for 8001536

2012-10-30 Thread Lance Andersen - Oracle
Hi,

This is a request for review of 
http://cr.openjdk.java.net/~lancea/8001536/webrev.00/.  This adds 
read/writeObject as well as clone methods to SerialXLob classes.

All SQE tests passed,  1 failure in the RowSet JCK/TCK tests due to a bug in 
the test that the TCK team is aware of and will address.  JDBC Unit tests all 
pass .


Best
Lance

Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: Review request for 8001536

2012-10-30 Thread Lance Andersen - Oracle
Hi Ulf,

The bug is described below, it is just adding the read/writeObject and clone 
methods.

Best
Lance
On Oct 30, 2012, at 2:18 PM, Ulf Zibis wrote:

 Hi all,
 
 please add the bug summary to the subject line.
 Bug  8001536 is not publicly visible :-(
 
 -Ulf
 
 Am 30.10.2012 17:25, schrieb Lance Andersen - Oracle:
 Hi,
 
 This is a request for review of 
 http://cr.openjdk.java.net/~lancea/8001536/webrev.00/.  This adds 
 read/writeObject as well as clone methods to SerialXLob classes.
 
 All SQE tests passed,  1 failure in the RowSet JCK/TCK tests due to a bug in 
 the test that the TCK team is aware of and will address.  JDBC Unit tests 
 all pass .
 
 
 Best
 Lance
 
 Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
 Oracle Java Engineering
 1 Network Drive
 Burlington, MA 01803
 lance.ander...@oracle.com
 
 
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: Review request for 8001536

2012-10-30 Thread Lance Andersen - Oracle
Hi Remi,


Thank you for the feedback

On Oct 30, 2012, at 2:05 PM, Remi Forax wrote:

 On 10/30/2012 05:25 PM, Lance Andersen - Oracle wrote:
 Hi,
 
 This is a request for review of 
 http://cr.openjdk.java.net/~lancea/8001536/webrev.00/.  This adds 
 read/writeObject as well as clone methods to SerialXLob classes.
 
 All SQE tests passed,  1 failure in the RowSet JCK/TCK tests due to a bug in 
 the test that the TCK team is aware of and will address.  JDBC Unit tests 
 all pass .
 
 Hi Lance.
 In SerialBlob and in SerialClob
 test (obj == null) is not necessary in equals, null instanceof X is always 
 false.

OK, thanks for the suggestion, I will make this change
 
 in hashCode, Objects.hash() allocate an array to pass arguments to 
 Arrays.hashCode() and box primitive values to Object.
 while this method is really convenient to use, each calls will allocate an 
 array and box the two values,
 the overhead seems to high here.
 This code should be equivalent:
return ((31 +Arrays.hashCode(buf)) * 31 +len) * 31 + origLen;
I can simplify hashCode to the what you have above, I liked the convenience 
method which is why I was using it. But happy to change it
 
 in clone, sb should not be initialized to null

I think it is OK as I have it as  HashMap does it similar to what I have done 
vs ArrayList which follows your suggestion.  Do we have a preferred practice or 
is this just a style choice?
 and the catch should be: throw new InternalError(e),

Given I am providing clone(), I did not see a reason to provide 
InternalError().  I have no strong preference but some java classes do and 
others do not (HashMap for example), so what is our preferred standard?
 this is the standard code you can see in clone.
 
 in readObject, the test (buf.length != len) can be done before decoding the 
 blob.

True, I can move it up.
 
 in writeObject, you set blob twice, which is weird,

my bad, I forgot to remove that.
 also I think that if blob is not Serializable,
 the code should throw an exception, so you should not use instanceof and let 
 s.writeFields()
 to throw NotSerializable exception.

This is intentional.  A Blob or Clob will not be serializable as its properties 
are unique to the database and it is created from an active Connection object.  

In the event someone actually tried to serialize this, I do not want it to fail 
just because someone passed in a LOB to instantiate this beast (note these 
methods should never have been created this way but this is way before my time).

In the unlikely event someone created their own wrapped Blob/Clob (which I 
cannot see why anyone would do), I am allowing for both for backwards 
compatibility.

Best
Lance
 
 cheers,
 Rémi
 
 
 
 Best
 Lance
 
 Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
 Oracle Java Engineering
 1 Network Drive
 Burlington, MA 01803
 lance.ander...@oracle.com
 
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: Review request for 8001536

2012-10-30 Thread Lance Andersen - Oracle
Point taken Ulf, thank you for the feedback and will follow your suggestion 
going forward , which I typically do, but I think I am still feeling the 
effects of being offline due to hurricane sandy :-(

Best
Lance


On Oct 30, 2012, at 2:50 PM, Ulf Zibis wrote:

 Thanks Lance.
 
 But having the subject of the request in clear text in the list view of the 
 email client would be a great help.
 
 -Ulf
 
 
 Am 30.10.2012 19:28, schrieb Lance Andersen - Oracle:
 Hi Ulf,
 
 The bug is described below, it is just adding the read/writeObject and clone 
 methods.
 
 Best
 Lance
 On Oct 30, 2012, at 2:18 PM, Ulf Zibis wrote:
 
 Hi all,
 
 please add the bug summary to the subject line.
 Bug  8001536 is not publicly visible :-(
 
 -Ulf
 
 Am 30.10.2012 17:25, schrieb Lance Andersen - Oracle:
 Hi,
 
 This is a request for review of 
 http://cr.openjdk.java.net/~lancea/8001536/webrev.00/.  This adds 
 read/writeObject as well as clone methods to SerialXLob classes.
 
 All SQE tests passed,  1 failure in the RowSet JCK/TCK tests due to a bug 
 in the test that the TCK team is aware of and will address.  JDBC Unit 
 tests all pass .
 
 
 Best
 Lance
 
 Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
 Oracle Java Engineering
 1 Network Drive
 Burlington, MA 01803
 lance.ander...@oracle.com
 
 
 
 
 Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
 Oracle Java Engineering
 1 Network Drive
 Burlington, MA 01803
 lance.ander...@oracle.com
 
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: Review request for 8001536 updated

2012-10-31 Thread Lance Andersen - Oracle
Here is revised webrev taking into account Remi's suggestions 
http://cr.openjdk.java.net/~lancea/8001536/webrev.01/


Best,
Lance


On Oct 30, 2012, at 2:05 PM, Remi Forax wrote:

 On 10/30/2012 05:25 PM, Lance Andersen - Oracle wrote:
 Hi,
 
 This is a request for review of 
 http://cr.openjdk.java.net/~lancea/8001536/webrev.00/.  This adds 
 read/writeObject as well as clone methods to SerialXLob classes.
 
 All SQE tests passed,  1 failure in the RowSet JCK/TCK tests due to a bug in 
 the test that the TCK team is aware of and will address.  JDBC Unit tests 
 all pass .
 
 Hi Lance.
 In SerialBlob and in SerialClob
 test (obj == null) is not necessary in equals, null instanceof X is always 
 false.
 
 in hashCode, Objects.hash() allocate an array to pass arguments to 
 Arrays.hashCode() and box primitive values to Object.
 while this method is really convenient to use, each calls will allocate an 
 array and box the two values,
 the overhead seems to high here.
 This code should be equivalent:
return ((31 +Arrays.hashCode(buf)) * 31 +len) * 31 + origLen;
 
 in clone, sb should not be initialized to null and the catch should be: throw 
 new InternalError(e),
 this is the standard code you can see in clone.
 
 in readObject, the test (buf.length != len) can be done before decoding the 
 blob.
 
 in writeObject, you set blob twice, which is weird, also I think that if 
 blob is not Serializable,
 the code should throw an exception, so you should not use instanceof and let 
 s.writeFields()
 to throw NotSerializable exception.
 
 cheers,
 Rémi
 
 
 
 Best
 Lance
 
 Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
 Oracle Java Engineering
 1 Network Drive
 Burlington, MA 01803
 lance.ander...@oracle.com
 
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: 8002120: ProblemList.txt updates (11/2012)

2012-11-01 Thread Lance Andersen - Oracle
looks fine

On Nov 1, 2012, at 5:12 PM, Alan Bateman wrote:

 
 I need a reviewer to remove 5 tests from the exclude list. 4 of the tests 
 were excluded temporarily during the perm gen removal work. The other one was 
 a compiler2 bug that is long fixed.
 
 While I was there I updated TEST.ROOT to add the top-level directories for 
 the client area to othervm.dirs. This ensures that the tests for those areas 
 is run in jtreg othervm mode even if samevm or agentvm mode is specified to 
 jtreg. If the tests in those areas are updated so that each test doesn't 
 require its own VM then they can be removed from the othervm.dirs list.
 
 The patch is trivial and inlined below.
 
 -Alan
 
 
 diff --git a/test/ProblemList.txt b/test/ProblemList.txt
 --- a/test/ProblemList.txt
 +++ b/test/ProblemList.txt
 @@ -136,12 +136,6 @@ java/lang/management/MemoryMXBean/ResetP
 
 # 7196801
 java/lang/management/MemoryMXBean/LowMemoryTest2.shgeneric-all
 -
 -# Exclude until the fix for 7195557 propagates widely.
 -java/lang/management/MemoryMXBean/CollectionUsageThresholdParallelGC.sh 
 generic-all
 -java/lang/management/MemoryMXBean/CollectionUsageThresholdSerialGC.sh 
 generic-all
 -java/lang/management/MemoryMXBean/MemoryTest.java  generic-all
 -java/lang/management/MemoryMXBean/MemoryTestAllGC.sh   generic-all
 
 # Exclude until hotspot/jdk repos are sync'd w.r.t. JAVA_MAX_SUPPORTED_VERSION
 # Needed when hotspot fix 7054345 is present.  Remove when the JDK source is
 @@ -334,9 +328,6 @@ sun/security/krb5/auto/MaxRetries.java
 
 # jdk_text
 
 -# 7196199
 -java/text/Bidi/Bug6665028.java  generic-all
 -
 
 
 # jdk_tools
 diff --git a/test/TEST.ROOT b/test/TEST.ROOT
 --- a/test/TEST.ROOT
 +++ b/test/TEST.ROOT
 @@ -6,7 +6,7 @@ keys=2d dnd i18n
 keys=2d dnd i18n
 
 # Tests that must run in othervm mode
 -othervm.dirs=java/rmi sun/rmi javax/management
 +othervm.dirs=java/awt java/beans java/rmi javax/accessibility javax/imageio 
 javax/sound javax/print javax/management com/sun/awt sun/awt sun/java2d 
 sun/pisces sun/rmi
 
 # Tests that cannot run concurrently
 exclusiveAccess.dirs=java/rmi/Naming sun/management/jmxremote sun/tools/jstatd
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Review request 8002212 - adding read/writeObject to additional SerialXXX classes

2012-11-02 Thread Lance Andersen - Oracle
This is similar to 8001536, just additional classes.

This adds read/writeObject, equals, clone methods to additional SerialXXX 
classes

SQE, JCK and JDBC Unit tests all pass.

The webrev can be viewed at http://cr.openjdk.java.net/~lancea/8002212/webrev.00

Best
Lance

Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: Review request 8002212 - adding read/writeObject to additional SerialXXX classes

2012-11-02 Thread Lance Andersen - Oracle
Hi Remi,

Thank you for the feedback
On Nov 2, 2012, at 7:42 PM, Remi Forax wrote:

 On 11/02/2012 11:57 PM, Lance Andersen - Oracle wrote:
 This is similar to 8001536, just additional classes.
 
 This adds read/writeObject, equals, clone methods to additional SerialXXX 
 classes
 
 SQE, JCK and JDBC Unit tests all pass.
 
 The webrev can be viewed at 
 http://cr.openjdk.java.net/~lancea/8002212/webrev.00
 
 Hi Lance,
 in SerialArray.equals(), I prefer
 
 return baseType == sa.baseType 
baseTypeName.equals(sa.baseTypeName)) 
Arrays.equals(elements, sa.elements);
 
 instead of
 
  if(baseType == sa.baseType  baseTypeName.equals(sa.baseTypeName)) {
  return Arrays.equals(elements, sa.elements);
  }
 

That is fine, I can make the change. 
 In SerialDataLink, do you really need readObject/writeObject given
 that you call the default implementations.

I thought about that but had decided to add them for consistency
 
 in SerialJavaObject, in equals, you should declare a local variable like in 
 SerialDataLink.equals,
 even if the local varialble is used once, it's more readable.

OK
 Also like in SerialArray.equals, you can do a return directly instead of 
 if(...) return true.
 in clone(), you can use the diamond syntax,
 sjo.chain = new Vector(chain);

Yeah, long story, but I forgot to reset to diamond syntax (will tell you over a 
beer sometime :-) )
 in setWarning(), you can use the diamond syntax as the original source does.
 and in readObject, you can use the diamond syntax too.
OK
 In readObject, you forget to throw an exception if there are some static 
 fields
 in getClass().getFields() as the constructor does
 (this code can be moved in a private static method).

I thought about that, but figured since it was already through that check when 
the object was created, I did not think it required repeating in the readObject 
method
 Also, you should add a comment that because you call getClass() on obj,
 there is an implicit null check.
Would it be better to put an null check in explicitly?
 
 This can be fixed as a separated bug or not but the code of
 method SerialJavaObject.getField() is weird, the code checks if fields can be 
 null,
 but fields is never null. Also, cloning the field array is perhaps a better 
 idea
 if the reflection implementation doesn't cache the array of fields.

I will do this in a separate bug, trying to keep things clear on the bug
 
 In SerialRef.equals, again, if(...) return should be transformed into return 
 ...

OK

Thank you again, will send an update webrev over the weekend

Best
Lance
 
 
 Best
 Lance
 
 cheers,
 Rémi
 
 
 Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
 Oracle Java Engineering
 1 Network Drive
 Burlington, MA 01803
 lance.ander...@oracle.com
 
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: Review request 8002212 - adding read/writeObject to additional SerialXXX classes -- Updated

2012-11-03 Thread Lance Andersen - Oracle
I revised the webrev, http://cr.openjdk.java.net/~lancea/8002212/webrev.01, 
taking into account the vast majority of Remi's suggestions.

I also added SerialStruct to the webrev.

Have a great weekend.

Best
Lance
On Nov 2, 2012, at 7:42 PM, Remi Forax wrote:

 On 11/02/2012 11:57 PM, Lance Andersen - Oracle wrote:
 This is similar to 8001536, just additional classes.
 
 This adds read/writeObject, equals, clone methods to additional SerialXXX 
 classes
 
 SQE, JCK and JDBC Unit tests all pass.
 
 The webrev can be viewed at 
 http://cr.openjdk.java.net/~lancea/8002212/webrev.00
 
 Hi Lance,
 in SerialArray.equals(), I prefer
 
 return baseType == sa.baseType 
baseTypeName.equals(sa.baseTypeName)) 
Arrays.equals(elements, sa.elements);
 
 instead of
 
  if(baseType == sa.baseType  baseTypeName.equals(sa.baseTypeName)) {
  return Arrays.equals(elements, sa.elements);
  }
 
 In SerialDataLink, do you really need readObject/writeObject given
 that you call the default implementations.
 
 in SerialJavaObject, in equals, you should declare a local variable like in 
 SerialDataLink.equals,
 even if the local varialble is used once, it's more readable.
 Also like in SerialArray.equals, you can do a return directly instead of 
 if(...) return true.
 in clone(), you can use the diamond syntax,
 sjo.chain = new Vector(chain);
 in setWarning(), you can use the diamond syntax as the original source does.
 and in readObject, you can use the diamond syntax too.
 In readObject, you forget to throw an exception if there are some static 
 fields
 in getClass().getFields() as the constructor does
 (this code can be moved in a private static method).
 Also, you should add a comment that because you call getClass() on obj,
 there is an implicit null check.
 
 This can be fixed as a separated bug or not but the code of
 method SerialJavaObject.getField() is weird, the code checks if fields can be 
 null,
 but fields is never null. Also, cloning the field array is perhaps a better 
 idea
 if the reflection implementation doesn't cache the array of fields.
 
 In SerialRef.equals, again, if(...) return should be transformed into return 
 ...
 
 
 Best
 Lance
 
 cheers,
 Rémi
 
 
 Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
 Oracle Java Engineering
 1 Network Drive
 Burlington, MA 01803
 lance.ander...@oracle.com
 
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: Review request 8002212 - adding read/writeObject to additional SerialXXX classes

2012-11-03 Thread Lance Andersen - Oracle

On Nov 3, 2012, at 11:14 AM, Remi Forax wrote:

 On 11/03/2012 01:46 AM, Lance Andersen - Oracle wrote:
 Hi Remi,
 
 [...]
 
 In SerialDataLink, do you really need readObject/writeObject given
 that you call the default implementations.
 I thought about that but had decided to add them for consistency
 
 The serialization implementation needs to create metadata associated with 
 readObject/writeObject,
 so if we can avoid to implement them, we reduce the runtime memory footprint 
 a little.
 I would prefer to have a comment saying that default serialization is Ok here.

OK,  I will just comment the methods out and add a comment as you suggest
 
 in SerialJavaObject, in equals, you should declare a local variable like in 
 SerialDataLink.equals,
 even if the local varialble is used once, it's more readable.
 OK
 Also like in SerialArray.equals, you can do a return directly instead of 
 if(...) return true.
 in clone(), you can use the diamond syntax,
 sjo.chain = new Vector(chain);
 Yeah, long story, but I forgot to reset to diamond syntax (will tell you 
 over a beer sometime :-) )
 
 sure, now or you have to visit Paris or I have to go to NY :)

Or Boston, where I am based or perhaps a JavaOne ;-)
 
 in setWarning(), you can use the diamond syntax as the original source does.
 and in readObject, you can use the diamond syntax too.
 OK
 In readObject, you forget to throw an exception if there are some static 
 fields
 in getClass().getFields() as the constructor does
 (this code can be moved in a private static method).
 I thought about that, but figured since it was already through that check 
 when the object was created, I did not think it required repeating in the 
 readObject method
 
 A serialization stream can be forged to encode a SerialJavaObject that 
 references an object that have a static field without creating a 
 SerialJavaObject in memory.

True and there are tests in the test suite that basically do that.  Anyways, I 
added that check in the revised webrev that I pushed earlier.
 
 Also, you should add a comment that because you call getClass() on obj,
 there is an implicit null check.
 Would it be better to put an null check in explicitly?
 
 As you prefer :)
 
 [...]
 
 Thank you again, will send an update webrev over the weekend
 
 Best
 Lance
 
 cheers,
 Rémi

Best
Lance

Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: Review request 8002212 - adding read/writeObject to additional SerialXXX classes -- Updated

2012-11-03 Thread Lance Andersen - Oracle

On Nov 3, 2012, at 11:34 AM, Remi Forax wrote:

 On 11/03/2012 03:11 PM, Lance Andersen - Oracle wrote:
 I revised the webrev, http://cr.openjdk.java.net/~lancea/8002212/webrev.01, 
 taking into account the vast majority of Remi's suggestions.
 
 in SerialJavaObject, hasStaticFields doesn't work, the original code doesn't 
 work because
 it only check for fields that are declared static but not for fields that are 
 by example public static.
 
 private static boolean hasStaticFields(Field[] fields) {
for (Field field : fields) {
if ( Modifier.isStatic(field.getModifiers())) {
return true;
}
}
return false;
}
 
 This may cause compatibility issue because despite the specification, the 
 original code
 will let objects that have a static field to be serialized.

I cannot make the change above as it breaks too many tests and I would prefer 
to go with the less is more scenario.  As I think I mentioned before, I do not 
think the original authors really thought through these classes and thankfully 
they are not used much, if at all.
 
 Also, in readObject, if obj is null, the code should throw an IOException 
 because
 it's not possible to create a SerialJavaObject with null has parameter 
 (because obj.getClass()
 that implictly checks null in the constructor).

I made the change to readObject.  I did not put an explicit check in the 
constructor but will do that under a separate bug

I also added the comment to SerialDataLink and removed the read/writeObject

http://cr.openjdk.java.net/~lancea/8002212/webrev.02

Best
Lance

 
 All other classes are Ok for me.
 
 
 I also added SerialStruct to the webrev.
 
 SerialStruct is Ok for me.
 
 
 Have a great weekend.
 
 Have a nice weekend too.
 
 
 Best
 Lance
 
 cheers,
 Rémi
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: [PATCH FOR REVIEW] ResourceManager.getApplicationResources() does not close InputStreams

2012-11-07 Thread Lance Andersen - Oracle
Is there a reason the patch was not created originally leveraging 
try-with-resoruces as it seems like the perfect candidate from the webrev?

I can create a bug for it, but I think I would prefer to see the patch take 
advantage of try-with-resoruces

Best
Lance
On Nov 7, 2012, at 3:30 PM, Andrew Hughes wrote:

 IcedTea bug: http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=1197
 
 com.sun.naming.internal.ResourceManager.getApplicationResources() does not 
 close the input streams it gets from helper.getResources() and 
 helper.getJavaHomeLibStream().  This patch:
 
 http://cr.openjdk.java.net/~andrew/pr1197/webrev.01/
 
 adds finally blocks to close these streams.  As noted in the original bug 
 report, this has been tested on a J2EE
 server (JBoss) without issue.
 
 I realise that there's a possibility that try-with-resources could be used 
 here, but the patch as posted
 is the one that was tested and I don't want to negate that testing by 
 changing it.
 
 Ok for tl?  If so, can I have a bug ID for this?
 
 Thanks,
 -- 
 Andrew :)
 
 Free Java Software Engineer
 Red Hat, Inc. (http://www.redhat.com)
 
 PGP Key: 248BDC07 (https://keys.indymedia.org/)
 Fingerprint = EC5A 1F5E C0AD 1D15 8F1F  8F91 3B96 A578 248B DC07
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: [PATCH FOR REVIEW] ResourceManager.getApplicationResources() does not close InputStreams

2012-11-07 Thread Lance Andersen - Oracle
The bug number is 8003120

Best
Lance
On Nov 7, 2012, at 3:30 PM, Andrew Hughes wrote:

 IcedTea bug: http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=1197
 
 com.sun.naming.internal.ResourceManager.getApplicationResources() does not 
 close the input streams it gets from helper.getResources() and 
 helper.getJavaHomeLibStream().  This patch:
 
 http://cr.openjdk.java.net/~andrew/pr1197/webrev.01/
 
 adds finally blocks to close these streams.  As noted in the original bug 
 report, this has been tested on a J2EE
 server (JBoss) without issue.
 
 I realise that there's a possibility that try-with-resources could be used 
 here, but the patch as posted
 is the one that was tested and I don't want to negate that testing by 
 changing it.
 
 Ok for tl?  If so, can I have a bug ID for this?
 
 Thanks,
 -- 
 Andrew :)
 
 Free Java Software Engineer
 Red Hat, Inc. (http://www.redhat.com)
 
 PGP Key: 248BDC07 (https://keys.indymedia.org/)
 Fingerprint = EC5A 1F5E C0AD 1D15 8F1F  8F91 3B96 A578 248B DC07
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: JDBC bug: Incorrect number of conflicts is reported by CachedRowSetWriter.writeData

2012-11-09 Thread Lance Andersen - Oracle
Frank,

If you can please post the bug info here, I will take a look at your patch

Best
Lance
On Nov 8, 2012, at 10:01 PM, Frank Ding wrote:

 Hi guys,
  We discovered a bug in CachedRowSetWriter.writeData method where incorrect 
 number of conflicts is reported.  I searched in Oracle bug database and no 
 similar record was found.  So I submitted a new one whose internal review ID 
 is 2376620.  A test case with code is illustrated in the bug submission that 
 leverages PostgreSQL server but the issue is platform independent and easy to 
 reproduce.
  I provided a patch review, available @
  http://cr.openjdk.java.net/~dingxmin/2376620/webrev.01/
  Is there anybody who is interested in patch and can also review bug 2376620? 
  Your reply is appreciated.
 
 Best regards,
 Frank
 
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: JDBC bug: Incorrect number of conflicts is reported by CachedRowSetWriter.writeData

2012-11-13 Thread Lance Andersen - Oracle
Hi Frank,


Thank you for the note 
If you could in the future, please provide  a  complete test program to repro 
the issue as it would save time with the reviews.  Ideally if the issue is not 
database specific it would be good to leverage Java DB as it is included within 
Oracle JDK

I will look at this sometime this week

Best
Lance
On Nov 12, 2012, at 9:25 PM, Frank Ding wrote:

 Hi Lance
  Thanks for your quick response.  Please find the bug info below.
 
  The problem:
  When CachedRowSetImpl.acceptChanges() is called, incorrect number of 
 conflicts, if any, is reported.  The number of conflicts is the actual number 
 of existing rows in database, which is the size of variable 'status' defined 
 in CachedRowSetWriter.writeData().  It's not the conflict number that is 
 supposed to be.
 
  Test case:
  The bug can be easily manifested in all SQL server environment. Here take 
 PostgreSQL for example.
  1. The sql script to create a table
 CREATE TABLE ressystem.roomdescription
 (
  roomdescription_id serial NOT NULL,
  roomdescription character varying NOT NULL,
  CONSTRAINT roomdescription_pkey PRIMARY KEY (roomdescription_id)
 )
 
  2.  Manually insert 3 rows
 (1, Test 1)
 (2, Test 2)
 (3, Test 3)
 
  3. Create a Java class that connects the established database and then 
 execute the following code snippet.
 String query = select roomdescription_id, roomdescription from 
 ressystem.roomdescription;
 Object[] values = {2, Test2};
 rs.setCommand(query);
 rs.execute(conn);
 rs.moveToInsertRow();
 for(int i=0; ivalues.length; i++) {
 rs.updateObject(i+1,values[i]);
 }
 rs.insertRow();
 rs.moveToCurrentRow();
 rs.acceptChanges(conn);
 
  4. An exception occurs with following output.
 javax.sql.rowset.spi.SyncProviderException: 4conflicts while synchronizing
at 
 com.sun.rowset.internal.CachedRowSetWriter.writeData(CachedRowSetWriter.java:412)
at com.sun.rowset.CachedRowSetImpl.acceptChanges(CachedRowSetImpl.java:880)
 
 5. In fact, there is only one conflicting row but 4 were reported.
 
 Best regards,
 Frank
 
 On 11/9/2012 7:41 PM, Lance Andersen - Oracle wrote:
 Frank,
 
 If you can please post the bug info here, I will take a look at your patch
 
 Best
 Lance
 On Nov 8, 2012, at 10:01 PM, Frank Ding wrote:
 
 Hi guys,
  We discovered a bug in CachedRowSetWriter.writeData method where incorrect 
 number of conflicts is reported.  I searched in Oracle bug database and no 
 similar record was found.  So I submitted a new one whose internal review 
 ID is 2376620.  A test case with code is illustrated in the bug submission 
 that leverages PostgreSQL server but the issue is platform independent and 
 easy to reproduce.
  I provided a patch review, available @
  http://cr.openjdk.java.net/~dingxmin/2376620/webrev.01/
  Is there anybody who is interested in patch and can also review bug 
 2376620?  Your reply is appreciated.
 
 Best regards,
 Frank
 
 
 
 
 Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
 Oracle Java Engineering
 1 Network Drive
 Burlington, MA 01803
 lance.ander...@oracle.com
 
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: RFR: 8003380 - Compiler warnings in logging test code

2012-11-14 Thread Lance Andersen - Oracle
looks Ok.


On Nov 14, 2012, at 4:15 PM, Jim Gish wrote:

 Please review 
 http://cr.openjdk.java.net/~jgish/Bug8003380-logging-test-warnings/ 
 http://cr.openjdk.java.net/%7Ejgish/Bug8003380-logging-test-warnings/
 
 These are simple changes to eliminate compiler warnings from 
 java.util.logging test code.
 
 Thanks,
   Jim
 
 -- 
 Jim Gish | Consulting Member of Technical Staff | +1.781.442.0304
 Oracle Java Platform Group | Core Libraries Team
 35 Network Drive
 Burlington, MA 01803
 jim.g...@oracle.com
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: javax.sql.rowset.serial.SerialBlob doesn't support free and getBinaryStream

2012-11-23 Thread Lance Andersen - Oracle
It is on my list.  to update the javadocs I need a ccc which I have not done 
yet and is needed as part of this change

On Nov 23, 2012, at 3:07 AM, Deven You wrote:

 Hi Lance,
 
 Is there any plan for this issue, if any could you update to me?
 
 Thanks a lot!
 On 10/29/2012 06:39 PM, Lance Andersen - Oracle wrote:
 Hi Deven,
 
 I will address the needed updates a bit later.
 
 Thank you for your input
 
 Best
 Lance
 On Oct 29, 2012, at 3:51 AM, Deven You wrote:
 
 Hi Alan,
 
 The Java Spec does not mention the thread safe for JDBC API. But I see the 
 other code in SerialBlob/SerialClob have not consider it.
 
 I think use buff == null to replace isFree is a good idea because it also 
 avoid the problem for the condition buf == null  isFree == false so we 
 won't need create a readObject method.
 
 Thanks for your suggestion for isFree, I will correct it later.
 
 Lance: How about your suggestion? Since you mentioned you will develop the 
 implementation yourself. I use my implementation mainly for the test cases. 
 But you may also take a look my implementation.
 
 Thanks a lot!
 
 On 09/21/2012 04:37 PM, Alan Bateman wrote:
 On 21/09/2012 04:21, Deven You wrote:
 Hi Lance,
 
 I am very busy with other work so I can't work with the 
 SerialBlob/SerialClob item for long time. I am very happy to refine the 
 current test case and create new tests for SerialClob.
 
 I have create a new webre[1] for this task, please review it.
 
 [1] http://cr.openjdk.java.net/~youdwei/OJDK-576/webrev.01/ 
 http://cr.openjdk.java.net/%7Eyoudwei/OJDK-576/webrev.01/
 
 PS: If the isFree is not transient, I want to know how we add this field 
 to the javadoc serialized form?
 I don't know very much about the rowset API and I can't see anything to 
 specify whether it is meant to be safe for use by concurrent threads. 
 There are clearly lots of issues here and implementing free introduces a 
 lot more, especially with the possibility of an asynchronous free or more 
 than one thread calling free at around the same time.
 
 Have you considered buf == null to mean that the resources are freed? 
 That might avoid needing to change the serialized form. Also as these 
 types are serializable it means you have to consider the case where you 
 deserialize to buf == null  isFree == false for example. On that point, 
 it looks to me that this code needs a readObject anyway (for several 
 reasons).
 
 A small point is that isFree is a odd name for a method that doesn't 
 return a boolean. If the patch goes ahead then I think it needs a better 
 name, ensureNotFree or requireNotFree or something like that.
 
 -Alan.
 
 
 
 Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
 Oracle Java Engineering 
 1 Network Drive 
 Burlington, MA 01803
 lance.ander...@oracle.com
 
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Adding field to BatchUpdateException

2012-11-24 Thread Lance Andersen - Oracle
Hi, 

For JDBC 4.2, I am adding methods to allow for larger update counts (request 
from JDBC driver vendors)  and because of this I have to tweak 
BatchUpdateException

The Statement interface has the method

int[] executeBatch()

I am planning to add

long[] executeLargeBatch().

To accomodate this change, I  also need to add a new field and the method 
getLargeUpdateCount to BatchUpdateException.

I have exchanged emails on this with Alan and he indicated that the changes 
seemed reasonable but to send a general email out to see if anything was  
missed from the serialization perspective.

I have added JDBC Unit tests to validate that the serialization/deserialization 
works between JDBC 4.1 and JDBC 4.2 and they run without a problem.


Best
Lance

new-host-2:sql lanceandersen$ diff BatchUpdateException.java 
~/NetBeansProjects/JDBC4.2/jdbc4.0/src/java/sql/
2c2
  * Copyright (c) 1998, 2011, Oracle and/or its affiliates. All rights 
reserved.
---
  * Copyright (c) 1998, 2012, Oracle and/or its affiliates. All rights 
 reserved.
27a28,31
 import java.io.IOException;
 import java.io.InvalidObjectException;
 import java.io.ObjectInputStream;
 import java.io.ObjectOutputStream;
83a88
   this.longUpdateCounts = (updateCounts == null) ? null : 
 copyUpdateCount(updateCounts);
192c197
 this((cause == null ? null : cause.toString()), null, 0, null, cause);
---
 this((cause == null ? null : cause.toString()), null, 0, (int[])null, 
 cause);
295a301
 this.longUpdateCounts = (updateCounts == null) ? null : 
 copyUpdateCount(updateCounts);
331c337,401
 
---
 
   /**
* Constructs a codeBatchUpdateException/code object initialized with
* a given codereason/code, codeSQLState/code, 
 codevendorCode/code
* codecause/code and codeupdateCounts/code.
* p
* This constructor should be used when the returned update count may exceed
* {@link Integer.MAX_VALUE}.
* p
* @param reason a description of the error
* @param SQLState an XOPEN or SQL:2003 code identifying the exception
* @param vendorCode an exception code used by a particular
* database vendor
* @param updateCounts an array of codelong/code, with each element
*indicating the update count, codeStatement.SUCCESS_NO_INFO/code or
* codeStatement.EXECUTE_FAILED/code for each SQL command in
* the batch for JDBC drivers that continue processing
* after a command failure; an update count or
* codeStatement.SUCCESS_NO_INFO/code for each SQL command in the batch
* prior to the failure for JDBC drivers that stop processing after a 
 command
* failure
* @param cause the underlying reason for this codeSQLException/code
* (which is saved for later retrieval by the codegetCause()/code 
 method);
* may be null indicating the cause is non-existent or unknown.
* @since 1.8
*/
   public BatchUpdateException(String reason, String SQLState, int vendorCode,
   long []updateCounts,Throwable cause) {
   super(reason, SQLState, vendorCode, cause);
   this.longUpdateCounts  = (updateCounts == null) ? null : 
 Arrays.copyOf(updateCounts, updateCounts.length);
   this.updateCounts = (longUpdateCounts == null) ? null : 
 copyUpdateCount(longUpdateCounts);
   }
  
   /**
* Retrieves the update count for each update statement in the batch
* update that executed successfully before this exception occurred.
* A driver that implements batch updates may or may not continue to
* process the remaining commands in a batch when one of the commands
* fails to execute properly. If the driver continues processing commands,
* the array returned by this method will have as many elements as
* there are commands in the batch; otherwise, it will contain an
* update count for each command that executed successfully before
* the codeBatchUpdateException/code was thrown.
* p
* This method should be used when the returned update count may exceed
* {@link Integer.MAX_VALUE}.
* p
* @return an array of codelong/code containing the update counts
* for the updates that were executed successfully before this error
* occurred.  Or, if the driver continues to process commands after an
* error, one of the following for every command in the batch:
* OL
* LIan update count
*  LIcodeStatement.SUCCESS_NO_INFO/code to indicate that the command
* executed successfully but the number of rows affected is unknown
*  LIcodeStatement.EXECUTE_FAILED/code to indicate that the command
* failed to execute successfully
* /OL
* @since 1.8
*/
   public long[] getLargeUpdateCounts() {
   return (longUpdateCounts == null) ? null : 
   Arrays.copyOf(longUpdateCounts, longUpdateCounts.length);
   }
   
337c407,414
   private final int[] updateCounts;
---
   private  int[] updateCounts;
 
   /**
* The array that describes the outcome of a batch execution.
* @serial

Re: Adding field to BatchUpdateException

2012-11-26 Thread Lance Andersen - Oracle
Hi Joe,

Thank you for the sanity check.

I had added the following to the top of the javadoc (still playing with the 
wording):

As of Java SE 8, the method getLargeUpdateCount has been added to provide 
support for update counts that may be exceed Integer.MAX_VALUE and returned by 
the method Statement.executeLargeBatch. A JDBC driver implementation is 
required to throw BatchUpdateException(String reason, String SQLState, int 
vendorCode, long []updateCounts,Throwable cause) if an error occurs during the 
the execution of Statement.executeLargeBatch. If Statement.executeLargeBatch is 
invoked it is recommended that getLargeUpdateCounts be called instead of 
getUpdateCounts in order to avoid a possible overflow of the integer update 
count.


Best
Lance

On Nov 26, 2012, at 1:51 AM, Joe Darcy wrote:

 Hi Lance,
 
 I don't see an obvious problem with the code, but I strongly suggest 
 documenting the correctness conditions regarding the updateCounts and 
 longUpdateCounts fields; I think that would ease reviewing the new 
 constructors and serialization code.
 
 Cheers,
 
 -Joe
 
 On 11/24/2012 2:05 PM, Lance Andersen - Oracle wrote:
 Hi,
 
 For JDBC 4.2, I am adding methods to allow for larger update counts (request 
 from JDBC driver vendors)  and because of this I have to tweak 
 BatchUpdateException
 
 The Statement interface has the method
 
 int[] executeBatch()
 
 I am planning to add
 
 long[] executeLargeBatch().
 
 To accomodate this change, I  also need to add a new field and the method 
 getLargeUpdateCount to BatchUpdateException.
 
 I have exchanged emails on this with Alan and he indicated that the changes 
 seemed reasonable but to send a general email out to see if anything was  
 missed from the serialization perspective.
 
 I have added JDBC Unit tests to validate that the 
 serialization/deserialization works between JDBC 4.1 and JDBC 4.2 and they 
 run without a problem.
 
 
 Best
 Lance
 
 new-host-2:sql lanceandersen$ diff BatchUpdateException.java 
 ~/NetBeansProjects/JDBC4.2/jdbc4.0/src/java/sql/
 2c2
   * Copyright (c) 1998, 2011, Oracle and/or its affiliates. All rights 
 reserved.
 ---
  * Copyright (c) 1998, 2012, Oracle and/or its affiliates. All rights 
 reserved.
 27a28,31
 import java.io.IOException;
 import java.io.InvalidObjectException;
 import java.io.ObjectInputStream;
 import java.io.ObjectOutputStream;
 83a88
   this.longUpdateCounts = (updateCounts == null) ? null : 
 copyUpdateCount(updateCounts);
 192c197
  this((cause == null ? null : cause.toString()), null, 0, null, 
 cause);
 ---
 this((cause == null ? null : cause.toString()), null, 0, 
 (int[])null, cause);
 295a301
 this.longUpdateCounts = (updateCounts == null) ? null : 
 copyUpdateCount(updateCounts);
 331c337,401
 
 ---
/**
* Constructs a codeBatchUpdateException/code object initialized with
* a given codereason/code, codeSQLState/code, 
 codevendorCode/code
* codecause/code and codeupdateCounts/code.
* p
* This constructor should be used when the returned update count may 
 exceed
* {@link Integer.MAX_VALUE}.
* p
* @param reason a description of the error
* @param SQLState an XOPEN or SQL:2003 code identifying the exception
* @param vendorCode an exception code used by a particular
* database vendor
* @param updateCounts an array of codelong/code, with each element
*indicating the update count, codeStatement.SUCCESS_NO_INFO/code or
* codeStatement.EXECUTE_FAILED/code for each SQL command in
* the batch for JDBC drivers that continue processing
* after a command failure; an update count or
* codeStatement.SUCCESS_NO_INFO/code for each SQL command in the 
 batch
* prior to the failure for JDBC drivers that stop processing after a 
 command
* failure
* @param cause the underlying reason for this codeSQLException/code
* (which is saved for later retrieval by the codegetCause()/code 
 method);
* may be null indicating the cause is non-existent or unknown.
* @since 1.8
*/
   public BatchUpdateException(String reason, String SQLState, int 
 vendorCode,
   long []updateCounts,Throwable cause) {
   super(reason, SQLState, vendorCode, cause);
   this.longUpdateCounts  = (updateCounts == null) ? null : 
 Arrays.copyOf(updateCounts, updateCounts.length);
   this.updateCounts = (longUpdateCounts == null) ? null : 
 copyUpdateCount(longUpdateCounts);
   }
 /**
* Retrieves the update count for each update statement in the batch
* update that executed successfully before this exception occurred.
* A driver that implements batch updates may or may not continue to
* process the remaining commands in a batch when one of the commands
* fails to execute properly. If the driver continues processing commands,
* the array returned by this method will have as many elements as
* there are commands in the batch; otherwise, it will contain

Re: Adding field to BatchUpdateException

2012-11-26 Thread Lance Andersen - Oracle
Hi Ulf,

Thank you for the suggestion

The current spec is silent on what happens if the update count overflows for 
any executeUpdate method.  Today the driver vendors just pass in their array of 
UpdateCounts (or status) to BatchException (or overflow the return count from 
executeUpdate)  if an error occurs at some point along the way.  I really would 
prefer to not change any existing behavior here at all.  If application 
programmers care about the update count, then they will switch to new xxxLarge 
methods.

Best
Lance
On Nov 26, 2012, at 5:10 PM, Ulf Zibis wrote:

 Hi Lance,
 
 I would throw an IllegalStateException if invoking e.g. getUpdateCounts on 
 integer overflow.
 
 -Ulf
 
 
 Am 26.11.2012 20:44, schrieb Lance Andersen - Oracle:
 Hi Joe,
 
 Thank you for the sanity check.
 
 I had added the following to the top of the javadoc (still playing with the 
 wording):
 
 As of Java SE 8, the method getLargeUpdateCount has been added to provide 
 support for update counts that may be exceed Integer.MAX_VALUE and returned 
 by the method Statement.executeLargeBatch. A JDBC driver implementation is 
 required to throw BatchUpdateException(String reason, String SQLState, int 
 vendorCode, long []updateCounts,Throwable cause) if an error occurs during 
 the the execution of Statement.executeLargeBatch. If 
 Statement.executeLargeBatch is invoked it is recommended that 
 getLargeUpdateCounts be called instead of getUpdateCounts in order to avoid 
 a possible overflow of the integer update count.
 
 
 Best
 Lance
 
 On Nov 26, 2012, at 1:51 AM, Joe Darcy wrote:
 
 Hi Lance,
 
 I don't see an obvious problem with the code, but I strongly suggest 
 documenting the correctness conditions regarding the updateCounts and 
 longUpdateCounts fields; I think that would ease reviewing the new 
 constructors and serialization code.
 
 Cheers,
 
 -Joe
 
 On 11/24/2012 2:05 PM, Lance Andersen - Oracle wrote:
 Hi,
 
 For JDBC 4.2, I am adding methods to allow for larger update counts 
 (request from JDBC driver vendors)  and because of this I have to tweak 
 BatchUpdateException
 
 The Statement interface has the method
 
 int[] executeBatch()
 
 I am planning to add
 
 long[] executeLargeBatch().
 
 To accomodate this change, I  also need to add a new field and the method 
 getLargeUpdateCount to BatchUpdateException.
 
 I have exchanged emails on this with Alan and he indicated that the 
 changes seemed reasonable but to send a general email out to see if 
 anything was  missed from the serialization perspective.
 
 I have added JDBC Unit tests to validate that the 
 serialization/deserialization works between JDBC 4.1 and JDBC 4.2 and they 
 run without a problem.
 
 
 Best
 Lance
 
 new-host-2:sql lanceandersen$ diff BatchUpdateException.java 
 ~/NetBeansProjects/JDBC4.2/jdbc4.0/src/java/sql/
 2c2
   * Copyright (c) 1998, 2011, Oracle and/or its affiliates. All rights 
 reserved.
 ---
  * Copyright (c) 1998, 2012, Oracle and/or its affiliates. All rights 
 reserved.
 27a28,31
 import java.io.IOException;
 import java.io.InvalidObjectException;
 import java.io.ObjectInputStream;
 import java.io.ObjectOutputStream;
 83a88
   this.longUpdateCounts = (updateCounts == null) ? null : 
 copyUpdateCount(updateCounts);
 192c197
  this((cause == null ? null : cause.toString()), null, 0, null, 
 cause);
 ---
 this((cause == null ? null : cause.toString()), null, 0, 
 (int[])null, cause);
 295a301
 this.longUpdateCounts = (updateCounts == null) ? null : 
 copyUpdateCount(updateCounts);
 331c337,401
 
 ---
/**
* Constructs a codeBatchUpdateException/code object initialized 
 with
* a given codereason/code, codeSQLState/code, 
 codevendorCode/code
* codecause/code and codeupdateCounts/code.
* p
* This constructor should be used when the returned update count may 
 exceed
* {@link Integer.MAX_VALUE}.
* p
* @param reason a description of the error
* @param SQLState an XOPEN or SQL:2003 code identifying the exception
* @param vendorCode an exception code used by a particular
* database vendor
* @param updateCounts an array of codelong/code, with each element
*indicating the update count, codeStatement.SUCCESS_NO_INFO/code or
* codeStatement.EXECUTE_FAILED/code for each SQL command in
* the batch for JDBC drivers that continue processing
* after a command failure; an update count or
* codeStatement.SUCCESS_NO_INFO/code for each SQL command in the 
 batch
* prior to the failure for JDBC drivers that stop processing after a 
 command
* failure
* @param cause the underlying reason for this codeSQLException/code
* (which is saved for later retrieval by the codegetCause()/code 
 method);
* may be null indicating the cause is non-existent or unknown.
* @since 1.8
*/
   public BatchUpdateException(String reason, String SQLState, int 
 vendorCode,
   long []updateCounts,Throwable cause) {
   super

Re: Request for Review : CR#8004015 : Add interface extends and defaults for basic functional interfaces

2012-11-30 Thread Lance Andersen - Oracle

On Nov 30, 2012, at 7:56 AM, Remi Forax wrote:

 On 11/30/2012 01:50 PM, Lance Andersen - Oracle wrote:
 On Nov 30, 2012, at 4:58 AM, Chris Hegarty wrote:
 
 
 On 30/11/2012 02:03, David Holmes wrote:
 On 30/11/2012 12:44 AM, Chris Hegarty wrote:
 On 11/29/2012 05:50 AM, David Holmes wrote:
 ...
 
 I don't agree that we need to describe what the default implementation
 does, for two reasons:
 
 1. Normal methods don't usually specify how they are implemented - it is
 an implementation detail. The default simply indicates that this
 method does have an implementation and you should expect that
 implementation to obey the contract of the method.
 
 2. It is not obvious to me that the JDK's choice for a default
 implementation has to be _the_ only possible implementation choice. In
 many/most cases there will be a very obvious choice, but that doesn't
 mean that all suppliers of OpenJDK classes have to be locked in to that
 choice.
 This is certainly interesting, and something I've wondered for a while
 now. If java.util.Iterator is to ever be fitted with a default
 implementation of remove ( to throw UnsupportedOperationException ),
 then it would clearly need to be part of the spec, and not an
 implementation detail of OpenJDK. Otherwise, what's the point, every
 developer will still have to implement it because they cannot be
 guaranteed of it's behavior.
 I think optional methods are a bit of a special case here because they
 don't have to work.
 
 It's the end user of a class that needs to understand if they can use
 remove() to actually do a removal. The developer of the class can
 inherit whatever default implementations Iterator provides, as long as
 they don't mind what they get. If they do mind ie they need a real
 remove(), then they will have to implement it themselves and in the
 process document that fact. The end user has to look at the docs for the
 concrete class and follow through to determine whether it's
 iterator().remove() is optional or not.
 
 Put another way, a default method is great for adding a new method to
 types that have not yet been revised to handle the new method. As a
 developer once you revise your class you should make a conscious
 implementation choice in my opinion and not rely on the default unless
 you truly don't care what it does.
 Sorry David, I've not been following lambda that closely, but (in my 
 opinion) if default methods do not, or cannot, have defined semantics then 
 I really think it is limiting. Maybe Iterator is a bad example, but I will 
 continue with it anyway. In many cases developers of iterator().remove() 
 want it to throw, if this is not defined in Iterator's default remove 
 method then every Iterator subclass will still have to define its own 
 remove that throws. For this particular case at least (if it were to ever 
 happen), I would like to see specification added to remove that defines the 
 default implementation.
 I had wondered about this as well and had a brief email exchange with Mike.  
 I thought a new javadoc tag might also be something to consider.
 
 @implementation ?
 
 
 For  JDBC,  I am thinking of leveraging default methods to throw a specific 
 exception (maybe IllegalStateException?) if the method must be implemented 
 by the driver vendor
 
 an UnsupportedOperationException is better for that.

Agree that could be a better choice
 
 or a SQLFeatureNotSupportedException for methods which may be optional based 
 on the backend support.
 
 Rémi
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: Request for Review : CR#8004015 : Add interface extends and defaults for basic functional interfaces

2012-11-30 Thread Lance Andersen - Oracle
The problem for an API such as JDBC is that the implementation is going to be 
specific to the driver and backend so providing a default implementation just 
won't work.  This allows existing drivers to compile as they finish their 
implementation and complete their migration to the new version of the API.  


On Nov 30, 2012, at 8:14 AM, Ricky Clarkson wrote:

 What is the benefit of throwing an IllegalStateException in a default method 
 over not providing any default method so that the compiler and runtime make 
 sure concrete subtypes provide an implementation?
 
 On Nov 30, 2012 9:54 AM, Lance Andersen - Oracle 
 lance.ander...@oracle.com wrote:
 
 On Nov 30, 2012, at 4:58 AM, Chris Hegarty wrote:
 
 
 
  On 30/11/2012 02:03, David Holmes wrote:
  On 30/11/2012 12:44 AM, Chris Hegarty wrote:
  On 11/29/2012 05:50 AM, David Holmes wrote:
  ...
 
  I don't agree that we need to describe what the default implementation
  does, for two reasons:
 
  1. Normal methods don't usually specify how they are implemented - it is
  an implementation detail. The default simply indicates that this
  method does have an implementation and you should expect that
  implementation to obey the contract of the method.
 
  2. It is not obvious to me that the JDK's choice for a default
  implementation has to be _the_ only possible implementation choice. In
  many/most cases there will be a very obvious choice, but that doesn't
  mean that all suppliers of OpenJDK classes have to be locked in to that
  choice.
 
  This is certainly interesting, and something I've wondered for a while
  now. If java.util.Iterator is to ever be fitted with a default
  implementation of remove ( to throw UnsupportedOperationException ),
  then it would clearly need to be part of the spec, and not an
  implementation detail of OpenJDK. Otherwise, what's the point, every
  developer will still have to implement it because they cannot be
  guaranteed of it's behavior.
 
  I think optional methods are a bit of a special case here because they
  don't have to work.
 
  It's the end user of a class that needs to understand if they can use
  remove() to actually do a removal. The developer of the class can
  inherit whatever default implementations Iterator provides, as long as
  they don't mind what they get. If they do mind ie they need a real
  remove(), then they will have to implement it themselves and in the
  process document that fact. The end user has to look at the docs for the
  concrete class and follow through to determine whether it's
  iterator().remove() is optional or not.
 
  Put another way, a default method is great for adding a new method to
  types that have not yet been revised to handle the new method. As a
  developer once you revise your class you should make a conscious
  implementation choice in my opinion and not rely on the default unless
  you truly don't care what it does.
 
  Sorry David, I've not been following lambda that closely, but (in my 
  opinion) if default methods do not, or cannot, have defined semantics then 
  I really think it is limiting. Maybe Iterator is a bad example, but I will 
  continue with it anyway. In many cases developers of iterator().remove() 
  want it to throw, if this is not defined in Iterator's default remove 
  method then every Iterator subclass will still have to define its own 
  remove that throws. For this particular case at least (if it were to ever 
  happen), I would like to see specification added to remove that defines the 
  default implementation.
 
 I had wondered about this as well and had a brief email exchange with Mike.  
 I thought a new javadoc tag might also be something to consider.
 
 For  JDBC,  I am thinking of leveraging default methods to throw a specific 
 exception (maybe IllegalStateException?) if the method must be implemented by 
 the driver vendor or a SQLFeatureNotSupportedException for methods which may 
 be optional based on the backend support.
 
  -Chris.
 
 
  But maybe we kid ourselves when we give this illusion of flexibility in
  implementation.
 
  David
 
  -Chris.
 
 
 
 Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
 Oracle Java Engineering
 1 Network Drive
 Burlington, MA 01803
 lance.ander...@oracle.com
 
 
 
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: JDBC bug: Incorrect number of conflicts is reported by CachedRowSetWriter.writeData

2012-12-03 Thread Lance Andersen - Oracle
I will get to it sometime within the next week.  Have some higher priority 
items to address first

Best
Lance
On Dec 3, 2012, at 3:17 AM, Frank Ding wrote:

 Hi Lance,
  Thanks for your clarification.  I created the test case you requested and 
 attached it in this email.  Please review it.  By the way, the new Oracle bug 
 (internal id 2376620) submitted by me several days ago seems not having been 
 reviewed.  Could you also help me on this?
 
 Best regards,
 Frank
 
 
 On 11/30/2012 8:40 PM, Lance Andersen - Oracle wrote:
 Hi Frank,
 
 Thank you for the email.  No we do not want tests that require database 
 access in jtreg.
 
 What I was trying to say, albeit not probably as clear as it could have been 
 is that it would be helpful to provide a complete example and to use Java DB 
 as the database if it is a generic data access issue as while it is not a 
 required part of the Java SE specification, we do provide it with the Oracle 
 JDK so it makes it easier to test against for developers vs finding an 
 instance of DB2, Sybase or even Oracle to test against.
 
 Best
 Lance
 On Nov 30, 2012, at 12:26 AM, Frank Ding wrote:
 
 Hi Lance,
  Sorry for late response and thanks for your comment.  You mean I can write 
 a jtreg test case that connects to Java DB?  I can do that.
 
 Best regards,
 Frank
 
 On 11/13/2012 10:13 PM, Lance Andersen - Oracle wrote:
 Hi Frank,
 
 
 Thank you for the note
 If you could in the future, please provide  a  complete test program to 
 repro the issue as it would save time with the reviews.  Ideally if the 
 issue is not database specific it would be good to leverage Java DB as it 
 is included within Oracle JDK
 
 I will look at this sometime this week
 
 Best
 Lance
 On Nov 12, 2012, at 9:25 PM, Frank Ding wrote:
 
 Hi Lance
  Thanks for your quick response.  Please find the bug info below.
 
  The problem:
  When CachedRowSetImpl.acceptChanges() is called, incorrect number of 
 conflicts, if any, is reported.  The number of conflicts is the actual 
 number of existing rows in database, which is the size of variable 
 'status' defined in CachedRowSetWriter.writeData().  It's not the 
 conflict number that is supposed to be.
 
  Test case:
  The bug can be easily manifested in all SQL server environment. Here 
 take PostgreSQL for example.
  1. The sql script to create a table
 CREATE TABLE ressystem.roomdescription
 (
  roomdescription_id serial NOT NULL,
  roomdescription character varying NOT NULL,
  CONSTRAINT roomdescription_pkey PRIMARY KEY (roomdescription_id)
 )
 
  2.  Manually insert 3 rows
 (1, Test 1)
 (2, Test 2)
 (3, Test 3)
 
  3. Create a Java class that connects the established database and then 
 execute the following code snippet.
 String query = select roomdescription_id, roomdescription from 
 ressystem.roomdescription;
 Object[] values = {2, Test2};
 rs.setCommand(query);
 rs.execute(conn);
 rs.moveToInsertRow();
 for(int i=0; ivalues.length; i++) {
 rs.updateObject(i+1,values[i]);
 }
 rs.insertRow();
 rs.moveToCurrentRow();
 rs.acceptChanges(conn);
 
  4. An exception occurs with following output.
 javax.sql.rowset.spi.SyncProviderException: 4conflicts while synchronizing
at 
 com.sun.rowset.internal.CachedRowSetWriter.writeData(CachedRowSetWriter.java:412)
at 
 com.sun.rowset.CachedRowSetImpl.acceptChanges(CachedRowSetImpl.java:880)
 
 5. In fact, there is only one conflicting row but 4 were reported.
 
 Best regards,
 Frank
 
 On 11/9/2012 7:41 PM, Lance Andersen - Oracle wrote:
 Frank,
 
 If you can please post the bug info here, I will take a look at your 
 patch
 
 Best
 Lance
 On Nov 8, 2012, at 10:01 PM, Frank Ding wrote:
 
 Hi guys,
  We discovered a bug in CachedRowSetWriter.writeData method where 
 incorrect number of conflicts is reported.  I searched in Oracle bug 
 database and no similar record was found.  So I submitted a new one 
 whose internal review ID is 2376620.  A test case with code is 
 illustrated in the bug submission that leverages PostgreSQL server but 
 the issue is platform independent and easy to reproduce.
  I provided a patch review, available @
  http://cr.openjdk.java.net/~dingxmin/2376620/webrev.01/
  Is there anybody who is interested in patch and can also review bug 
 2376620?  Your reply is appreciated.
 
 Best regards,
 Frank
 
 
 Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
 Oracle Java Engineering
 1 Network Drive
 Burlington, MA 01803
 lance.ander...@oracle.com
 
 
 Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
 Oracle Java Engineering
 1 Network Drive
 Burlington, MA 01803
 lance.ander...@oracle.com
 
 
 
 Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
 Oracle Java Engineering
 1 Network Drive
 Burlington, MA 01803
 lance.ander...@oracle.com
 
 
/*
 * Copyright (c) 2012 Oracle and/or its affiliates. All rights reserved.
 * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
 *
 * This code is free software; you can

Re: Request for Review : CR#8004015 : [2nd pass] Add interface extends and defaults for basic functional interfaces

2012-12-05 Thread Lance Andersen - Oracle
I am still wondering if we need some sort of javadoc tag for default 
implementations so that it will stand out better and allow us to be consistent 
with how we specify this across Java SE and other APIs that leverage default 
methods.

Has any thought been given to this?

Best
Lance

On Dec 5, 2012, at 12:47 AM, Mike Duigou wrote:

 Hello all;
 
 I have updated the proposed patch. The changes primarily add class and method 
 documentation regarding handling of null for the primitive specializations.
 
 http://cr.openjdk.java.net/~mduigou/8004015/1/webrev/
 http://cr.openjdk.java.net/~mduigou/8004015/1/specdiff/java/util/function/package-summary.html
 
 I've also reformatted the source for the default methods.
 
 Mike
 
 
 On Nov 26 2012, at 18:12 , Mike Duigou wrote:
 
 Hello all;
 
 In the original patch which added the basic lambda functional interfaces, 
 CR#8001634 [1], none of the interfaces extended other interfaces. The reason 
 was primarily that the javac compiler did not, at the time that 8001634 was 
 proposed, support extension methods. The compiler now supports adding of 
 method defaults so this patch improves the functional interfaces by filing 
 in the inheritance hierarchy. 
 
 Adding the parent interfaces and default methods allows each functional 
 interface to be used in more places. It is especially important for the 
 functional interfaces which support primitive types, IntSupplier, 
 IntFunction, IntUnaryOperator, IntBinaryOperator, etc. We expect that 
 eventually standard implementations of these interfaces will be provided for 
 functions like max, min, sum, etc. By extending the reference oriented 
 functional interfaces such as Function, the primitive implementations can be 
 used with the boxed primitive types along with the primitive types for which 
 they are defined.
 
 The patch to add parent interfaces and default methods can be found here:
 
 http://cr.openjdk.java.net/~mduigou/8004015/0/webrev/
 http://cr.openjdk.java.net/~mduigou/8004015/0/specdiff/java/util/function/package-summary.html
 
 Mike
 
 [1] http://hg.openjdk.java.net/jdk8/tl/jdk/rev/c2e80176a697
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



signatures that are recorded for default methods

2012-12-06 Thread Lance Andersen - Oracle
Folks,

Will the signatures for interfaces that are recorded by the TCKs for interfaces 
record the fact that a method includes a default method? or will it just record 
the method definition?

I am assuming it will, but I know there has been discussion that a implementor 
could choose a different default implementation on one of the recent threads 
that was up for review.

I am still trying to understand what our guidelines are, if any for documenting 
the behavior of the supplied default methods given the javadocs are part of the 
specification for many APIs (and in some case the only spec).

Best
Lance

Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: Request for review: JDK-8004337: java/sql tests aren't run in test/Makefile

2012-12-06 Thread Lance Andersen - Oracle
Looks fine Rob
On Dec 6, 2012, at 4:22 PM, Rob McKenna wrote:

 Hi folks,
 
 There's a missing folder in the jdk_other test target:
 
 http://cr.openjdk.java.net/~robm/8004337/webrev.01/ 
 http://cr.openjdk.java.net/%7Erobm/8004337/webrev.01/
 
-Rob


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: Review needed: 8004374 : Fwd: JDBC bug: Incorrect number of conflicts is reported by CachedRowSetWriter.writeData

2012-12-10 Thread Lance Andersen - Oracle
Hi Frank,

As I explained in one of my earlier emails,  tests that require a database will 
not be added to jtreg.  I have a unit test suite which i use for that but that 
is not external


Best
Lance
On Dec 10, 2012, at 2:44 AM, Frank Ding wrote:

 Hi Lance,
  The code refactory looks good.  By the way, the newly added unit test is not 
 jtreg test case?
 
 Best regards,
 Frank
 
 On 12/5/2012 4:38 AM, Lance Andersen - Oracle wrote:
 All,
 
 Attached is the patch for:  8004374 based off the issue that Frank reported.
 
 for http://cr.openjdk.java.net/~lancea/8004374/webrev.00/ 
 http://cr.openjdk.java.net/%7Elancea/8004374/webrev.00/
 
 The TCK, SQE and the JDBC Unit Tests run clean.  I added a new Unit Test to 
 validate the issue.
 
 Frank, I did not use your fix as I was able to clean the code up a bit more 
 and get rid of more crud while addressing it.  It is similar though.
 
 Best
 Lance
 
 http://oracle.com/us/design/oracle-email-sig-198324.gif
 
 
 http://oracle.com/us/design/oracle-email-sig-198324.gif
 http://oracle.com/us/design/oracle-email-sig-198324.gifLance Andersen| 
 Principal Member of Technical Staff | +1.781.442.2037
 Oracle Java Engineering
 1 Network Drive
 Burlington, MA 01803
 lance.ander...@oracle.com mailto:lance.ander...@oracle.com
 
 
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Review request 8004357: Implement various methods in SerialBlob/Clob/Array and specify Thread Safety

2012-12-11 Thread Lance Andersen - Oracle

Need a reviewer for 8004357:Implement various methods in 
SerialBlob/Clob/Array and specify Thread Safety

This defines thread safety adds missing methods to SerialBlob/Clob/Array


The CCC request has been reviewed.  The changes uncovered a couple of bugs in 
the JCK which the JCK team is going to address in the JCK and RowSet TCK.  JDBC 
Unit tests and SQE tests all continue to pass.

http://cr.openjdk.java.net/~lancea/8004357/webrev.00/

Best
Lance


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: RFR: 8005051: default methods for Iterator

2012-12-14 Thread Lance Andersen - Oracle

On Dec 14, 2012, at 7:28 AM, Alan Bateman wrote:

 On 14/12/2012 01:24, Akhil Arora wrote:
 As part of the library lambdafication, this patch adds a forEach default 
 method to Iterator, and converts remove() into a default method so that 
 implementations of Iterator no longer have to override remove if they desire 
 the default behavior, which is to throw an UnsupportedOperationException.
 
 http://cr.openjdk.java.net/~akhil/8005051.0/webrev/
 I looked at the changes to Iterator, a few minor comments:
 
 I think it would help to change This default implementation to The default 
 implementation, it makes it more obviously normative (and so testable) and 
 might help for sub-types that override the method but don't inherit the 
 javadoc.
 
 I assume the generated javadoc ends as a long paragraph, did you consider 
 putting in p tags to make it a bit easier on the reader, minimally put the 
 description of the default implementation isn't own paragraph.

This is why i am a proponent of adding a javadoc tag for default methods so 
that everyone is consistent in where/how we document the default method.  I 
worry if we do not developers will place this info in various places making it 
easy to overlook.
 
 Should Collection have a lowercase c as it may be an iterator over a 
 collection that is not a java.util.Collection?
 
 In forEach then it may be smoother to change ... execution subsequent ... 
 to ... execution then subsequent 
 
 As per the other thread, if new methods are coming with the public modifier 
 then I think it should be added to the existing methods.
 
 -Alan.
 
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: RFR: (jaxp) 8003260 : some fields should be package protected

2012-12-14 Thread Lance Andersen - Oracle
Hi Joe,

Shouldn't this also be private:

 static final char [] xmlDecl = {'','?','x','m','l'};

otherwise it is fine
Best
Lance
On Dec 14, 2012, at 1:33 PM, Joe Wang wrote:

 Hi,
 
 This is one of the three [findbug] issues. I've checked with Drew. None of 
 them are vulnerabilities. Nonetheless, the fields should have been private or 
 package private.
 
 webrev:
 http://cr.openjdk.java.net/~joehw/7u12/8003260/webrev/
 
 Test:
 No new test. Existing regression tests passed.
 
 Thanks,
 Joe


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: RFR: (jaxp) 8003260 : some fields should be package protected

2012-12-14 Thread Lance Andersen - Oracle
Thanks Joe.  maybe a quick comment would help in the code could be useful

Best
Lance
On Dec 14, 2012, at 2:49 PM, Joe Wang wrote:

 
 
 On 12/14/2012 10:36 AM, Lance Andersen - Oracle wrote:
 
 Hi Joe,
 
 Shouldn't this also be private:
 
  static final char [] xmlDecl = {'','?','x','m','l'};
 
 A subclass in the same package, XMLDocumentScannerImpl, referred to it. 
 That's why I made it package private.
 
 Joe
 
 otherwise it is fine
 Best
 Lance
 On Dec 14, 2012, at 1:33 PM, Joe Wang wrote:
 
 Hi,
 
 This is one of the three [findbug] issues. I've checked with Drew. None of 
 them are vulnerabilities. Nonetheless, the fields should have been private 
 or package private.
 
 webrev:
 http://cr.openjdk.java.net/~joehw/7u12/8003260/webrev/
 
 Test:
 No new test. Existing regression tests passed.
 
 Thanks,
 Joe
 
 
 Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
 Oracle Java Engineering 
 1 Network Drive 
 Burlington, MA 01803
 lance.ander...@oracle.com
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: RFR: (jaxp) 8003260 : some fields should be package protected

2012-12-14 Thread Lance Andersen - Oracle
+1
On Dec 14, 2012, at 3:43 PM, Joe Wang wrote:

 Thanks.  I added a comment.  Here's the webrev again:
 http://cr.openjdk.java.net/~joehw/7u12/8003260/webrev/
 
 Best
 Joe
 
 On 12/14/2012 11:52 AM, Lance Andersen - Oracle wrote:
 
 Thanks Joe.  maybe a quick comment would help in the code could be useful
 
 Best
 Lance
 On Dec 14, 2012, at 2:49 PM, Joe Wang wrote:
 
 
 
 On 12/14/2012 10:36 AM, Lance Andersen - Oracle wrote:
 
 Hi Joe,
 
 Shouldn't this also be private:
 
  static final char [] xmlDecl = {'','?','x','m','l'};
 
 A subclass in the same package, XMLDocumentScannerImpl, referred to it. 
 That's why I made it package private.
 
 Joe
 otherwise it is fine
 Best
 Lance
 On Dec 14, 2012, at 1:33 PM, Joe Wang wrote:
 
 Hi,
 
 This is one of the three [findbug] issues. I've checked with Drew. None 
 of them are vulnerabilities. Nonetheless, the fields should have been 
 private or package private.
 
 webrev:
 http://cr.openjdk.java.net/~joehw/7u12/8003260/webrev/
 
 Test:
 No new test. Existing regression tests passed.
 
 Thanks,
 Joe
 
 
 Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
 Oracle Java Engineering 
 1 Network Drive 
 Burlington, MA 01803
 lance.ander...@oracle.com
 
 
 
 Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
 Oracle Java Engineering 
 1 Network Drive 
 Burlington, MA 01803
 lance.ander...@oracle.com
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: javax.sql.rowset.serial.SerialBlob doesn't support free and getBinaryStream

2013-01-10 Thread Lance Andersen - Oracle
Deven,

This was pushed a while ago you should have seen the putback on the alias

See http://hg.openjdk.java.net/jdk8/tl/jdk/rev/7a8978a5bb6e

Best
Lance


On Jan 10, 2013, at 12:49 AM, Deven You wrote:

 Hi Lance,
 
 Is there any update for this issue. If you have anything I can do, please let 
 me know.
 
 Thanks a lot!
 On 11/24/2012 12:45 AM, Lance Andersen - Oracle wrote:
 It is on my list.  to update the javadocs I need a ccc which I have not done 
 yet and is needed as part of this change
 
 On Nov 23, 2012, at 3:07 AM, Deven You wrote:
 
 Hi Lance,
 
 Is there any plan for this issue, if any could you update to me?
 
 Thanks a lot!
 On 10/29/2012 06:39 PM, Lance Andersen - Oracle wrote:
 Hi Deven,
 
 I will address the needed updates a bit later.
 
 Thank you for your input
 
 Best
 Lance
 On Oct 29, 2012, at 3:51 AM, Deven You wrote:
 
 Hi Alan,
 
 The Java Spec does not mention the thread safe for JDBC API. But I see 
 the other code in SerialBlob/SerialClob have not consider it.
 
 I think use buff == null to replace isFree is a good idea because it also 
 avoid the problem for the condition buf == null  isFree == false so we 
 won't need create a readObject method.
 
 Thanks for your suggestion for isFree, I will correct it later.
 
 Lance: How about your suggestion? Since you mentioned you will develop 
 the implementation yourself. I use my implementation mainly for the test 
 cases. But you may also take a look my implementation.
 
 Thanks a lot!
 
 On 09/21/2012 04:37 PM, Alan Bateman wrote:
 On 21/09/2012 04:21, Deven You wrote:
 Hi Lance,
 
 I am very busy with other work so I can't work with the 
 SerialBlob/SerialClob item for long time. I am very happy to refine the 
 current test case and create new tests for SerialClob.
 
 I have create a new webre[1] for this task, please review it.
 
 [1] http://cr.openjdk.java.net/~youdwei/OJDK-576/webrev.01/ 
 http://cr.openjdk.java.net/%7Eyoudwei/OJDK-576/webrev.01/
 
 PS: If the isFree is not transient, I want to know how we add this 
 field to the javadoc serialized form?
 I don't know very much about the rowset API and I can't see anything to 
 specify whether it is meant to be safe for use by concurrent threads. 
 There are clearly lots of issues here and implementing free introduces a 
 lot more, especially with the possibility of an asynchronous free or 
 more than one thread calling free at around the same time.
 
 Have you considered buf == null to mean that the resources are freed? 
 That might avoid needing to change the serialized form. Also as these 
 types are serializable it means you have to consider the case where you 
 deserialize to buf == null  isFree == false for example. On that 
 point, it looks to me that this code needs a readObject anyway (for 
 several reasons).
 
 A small point is that isFree is a odd name for a method that doesn't 
 return a boolean. If the patch goes ahead then I think it needs a better 
 name, ensureNotFree or requireNotFree or something like that.
 
 -Alan.
 
 
 
 Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
 Oracle Java Engineering 
 1 Network Drive 
 Burlington, MA 01803
 lance.ander...@oracle.com
 
 
 
 
 Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
 Oracle Java Engineering 
 1 Network Drive 
 Burlington, MA 01803
 lance.ander...@oracle.com
 
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: javax.sql.rowset.serial.SerialBlob doesn't support free and getBinaryStream

2013-01-10 Thread Lance Andersen - Oracle

The JCK and RowSet TCK will have base tests to validate.  I have a separate set 
of unit tests for JDBC which includes more detailed tests than what you had 
suggested as well as end to end tests.

Best
Lance 
On Jan 10, 2013, at 8:24 AM, Deven You wrote:

 Hi Lance,
 
 I am glad to see the patch for implementing these methods are committed.
 
 Do you have a plan to add the test cases[1] I created too?
 
 Thanks a lot!
 
 [1]http://cr.openjdk.java.net/~youdwei/OJDK-576/webrev.01/
 
 
 On 01/10/2013 08:31 PM, Lance Andersen - Oracle wrote:
 Deven,
 
 This was pushed a while ago you should have seen the putback on the alias
 
 See http://hg.openjdk.java.net/jdk8/tl/jdk/rev/7a8978a5bb6e
 
 Best
 Lance
 
 
 On Jan 10, 2013, at 12:49 AM, Deven You wrote:
 
 Hi Lance,
 
 Is there any update for this issue. If you have anything I can do, please 
 let me know.
 
 Thanks a lot!
 On 11/24/2012 12:45 AM, Lance Andersen - Oracle wrote:
 It is on my list.  to update the javadocs I need a ccc which I have not 
 done yet and is needed as part of this change
 
 On Nov 23, 2012, at 3:07 AM, Deven You wrote:
 
 Hi Lance,
 
 Is there any plan for this issue, if any could you update to me?
 
 Thanks a lot!
 On 10/29/2012 06:39 PM, Lance Andersen - Oracle wrote:
 Hi Deven,
 
 I will address the needed updates a bit later.
 
 Thank you for your input
 
 Best
 Lance
 On Oct 29, 2012, at 3:51 AM, Deven You wrote:
 
 Hi Alan,
 
 The Java Spec does not mention the thread safe for JDBC API. But I see 
 the other code in SerialBlob/SerialClob have not consider it.
 
 I think use buff == null to replace isFree is a good idea because it 
 also avoid the problem for the condition buf == null  isFree == false 
 so we won't need create a readObject method.
 
 Thanks for your suggestion for isFree, I will correct it later.
 
 Lance: How about your suggestion? Since you mentioned you will develop 
 the implementation yourself. I use my implementation mainly for the 
 test cases. But you may also take a look my implementation.
 
 Thanks a lot!
 
 On 09/21/2012 04:37 PM, Alan Bateman wrote:
 On 21/09/2012 04:21, Deven You wrote:
 Hi Lance,
 
 I am very busy with other work so I can't work with the 
 SerialBlob/SerialClob item for long time. I am very happy to refine 
 the current test case and create new tests for SerialClob.
 
 I have create a new webre[1] for this task, please review it.
 
 [1]http://cr.openjdk.java.net/~youdwei/OJDK-576/webrev.01/  
 http://cr.openjdk.java.net/%7Eyoudwei/OJDK-576/webrev.01/
 
 PS: If the isFree is not transient, I want to know how we add this 
 field to the javadoc serialized form?
 I don't know very much about the rowset API and I can't see anything 
 to specify whether it is meant to be safe for use by concurrent 
 threads. There are clearly lots of issues here and implementing free 
 introduces a lot more, especially with the possibility of an 
 asynchronous free or more than one thread calling free at around the 
 same time.
 
 Have you considered buf == null to mean that the resources are 
 freed? That might avoid needing to change the serialized form. Also as 
 these types are serializable it means you have to consider the case 
 where you deserialize to buf == null  isFree == false for example. 
 On that point, it looks to me that this code needs a readObject anyway 
 (for several reasons).
 
 A small point is that isFree is a odd name for a method that doesn't 
 return a boolean. If the patch goes ahead then I think it needs a 
 better name, ensureNotFree or requireNotFree or something like that.
 
 -Alan.
 
 
 
 Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
 Oracle Java Engineering
 1 Network Drive
 Burlington, MA 01803
 lance.ander...@oracle.com
 
 
 
 http://oracle.com/us/design/oracle-email-sig-198324.gif
 http://oracle.com/us/design/oracle-email-sig-198324.gifLance Andersen| 
 Principal Member of Technical Staff | +1.781.442.2037
 Oracle Java Engineering
 1 Network Drive
 Burlington, MA 01803
 lance.ander...@oracle.com mailto:lance.ander...@oracle.com
 
 
 
 http://oracle.com/us/design/oracle-email-sig-198324.gif
 http://oracle.com/us/design/oracle-email-sig-198324.gifLance Andersen| 
 Principal Member of Technical Staff | +1.781.442.2037
 Oracle Java Engineering
 1 Network Drive
 Burlington, MA 01803
 lance.ander...@oracle.com mailto:lance.ander...@oracle.com
 
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Review request 8005080: JDBC 4.2

2013-01-10 Thread Lance Andersen - Oracle
The following webrev has the bulk of  the JDBC 4.2 changes: 
http://cr.openjdk.java.net/~lancea/8005080/webrev.00/

There will be additional updates to java.sql.Date/TIme/Timestamp (by Sherman) 
once JSR 310 is integrated to aide in moving to and from the new date time 
datatypes.  

I will also probably have one more set of minor changes once 310 is integrated.

The CCC has been approved for these changes the specdiff of the javadocs is 
still available at http://cr.openjdk.java.net/~lancea/8005080/specdiffs.02/ 
(note some of the changes highlighted were put back previously)


Best
Lance

Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: RFR: javax.xml.validation: Using ServiceLoader to load JAXP schema factories (7169894: JAXP Plugability Layer: using service loader)

2013-01-11 Thread Lance Andersen - Oracle

On Jan 11, 2013, at 2:05 PM, Joe Wang wrote:

 
 
 On 1/11/2013 8:58 AM, Alan Bateman wrote:
 On 09/01/2013 14:28, Daniel Fuchs wrote:
 Hi,
 
 Here is a new webrev in the series that addresses using ServiceLoader in
 JAXP for JDK 8.
 
 7169894: JAXP Plugability Layer: using service loader
 
 This changeset addresses modification in the javax.xml.validation
 package.
 
 It is a bit more complex than the changes required for the other
 packages because the newInstance methods takes an additional
 schemaLanguage parameter, and therefore we need to loop over
 the providers in order to find one that supports the language.
 
 
 http://cr.openjdk.java.net/~dfuchs/JDK-7169894/javax.xml.validation/webrev.00/
  
 
 Also this particular package did not have any specific configuration
 error to throw in case of misconfiguration (the xpath package in which
 the logic is very similar had one for instance), so we're adding a new
 SchemaFactoryConfigurationError for that purpose.
 I've taken an initial look at this and I'm wondering about 
 SchemeFactory.newInstance throwing SchemaFactoryConfigurationError. 
 Technically this is an incompatible change but in practical terms it may not 
 be concern as this provider interface may not be used very much.
 
 Joe Wang - have you come across SchemaFactory implementations, I'm trying to 
 get a feel for how much this is used, if ever.
 
 I don't have any data on how much the service mechanism may be used, Xerces 
 would surely be the one most frequently used. I'm more concerned with the 
 spec change that would require TCK change (the addition of  
 SchemaFactoryConfigurationError related tests). Would that require MR? We 
 probably need to run it with the JCK engineers.

If you are changing the exception, then you would need an MR for this and want 
to update any TCK/JCK tests (or add tests)
 
 Joe
 
 
 -Alan


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



review request 8006139, add missing methods to javax.sql.rowset.serial.SQLInput/OutputImpl

2013-01-12 Thread Lance Andersen - Oracle
Hi 
This is a review request for 8006139 which adds missing methods to 
SQLInput/Output

The webrev can be found at http://cr.openjdk.java.net/~lancea/8006139/webrev.00/

best
Lance

Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: review request 8006139, add missing methods to javax.sql.rowset.serial.SQLInput/OutputImpl

2013-01-15 Thread Lance Andersen - Oracle
Here is a revision

http://cr.openjdk.java.net/~lancea/8006139/webrev.01

I still have to enter the JBS entry for the javadoc clarifications (and I also 
found another javadoc issue that was due to incorrect cut  paste when the code 
was written) and ccc request

As i mentioned in an earlier thread, these classes are hardly ever, if at all 
used and would only be used when UDTs are used and the majority of databases do 
not support this.  


Best
lance
On Jan 14, 2013, at 5:11 AM, Alan Bateman wrote:

 On 13/01/2013 23:51, Lance Andersen - Oracle wrote:
 :
 
 One other thing is that the CCE has a side-effect in that it consumes 
 the next attribute. The methods could be changed to peek at the next 
 attribute but that wouldn't work without synchronization or making it 
 clear in the spec that the it is not a thread-safe implementation of 
 SQLInput.
 I really want to keep the changes to the bare minimum here as this and the 
 other serial classes are hardly, if ever used at all.
 I understand, but if you add a catch-all in the class description to cover 
 the CCE case then this could be part of the same paragraph.
 
 -Alan


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: 8006344: Broken javadoc link in javax.lang.model.element.Element

2013-01-15 Thread Lance Andersen - Oracle
+1
On Jan 15, 2013, at 12:59 PM, Chris Hegarty wrote:

 Minor oversight in the changes from 7193719: Support repeating annotations 
 in javax.lang.model.
 
 ~/repos/jdk8/tl/adder/build/solaris-i586/impsrc/javax/lang/model/element/Element.java:199:
  warning - Tag @see: can't find getAnnotation() in 
 javax.lang.model.element.Element
 
 I only noticed when building javadocs for another issue, thankfully we don't 
 have many warnings here ;-)
 
 
 diff -r d54b4a091450 src/share/classes/javax/lang/model/element/Element.java
 --- a/src/share/classes/javax/lang/model/element/Element.java   Mon Jan 14 
 14:17:25 2013 -0800
 +++ b/src/share/classes/javax/lang/model/element/Element.java   Tue Jan 15 
 17:55:17 2013 +
 @@ -188,7 +188,7 @@ public interface Element {
  * type if present on this element, else an empty array
  *
  * @see #getAnnotationMirrors()
 - * @see #getAnnotation()
 + * @see #getAnnotation(java.lang.Class)
  * @see java.lang.reflect.AnnotatedElement#getAnnotations
  * @see EnumConstantNotPresentException
  * @see AnnotationTypeMismatchException
 
 -Chris.


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: review request 8006139, add missing methods to javax.sql.rowset.serial.SQLInput/OutputImpl

2013-01-15 Thread Lance Andersen - Oracle
Thank you Ulf.

I deleted the extra line on 579

Best
Lance
On Jan 15, 2013, at 3:45 PM, Ulf Zibis wrote:

 Looks great!
 
 Little nit:
 SQLOutputImpl.java line 579 could be dropped.
 
 -Ulf
 
 
 Am 15.01.2013 17:48, schrieb Lance Andersen - Oracle:
 Here is a revision
 
 http://cr.openjdk.java.net/~lancea/8006139/webrev.01
 
 I still have to enter the JBS entry for the javadoc clarifications (and I 
 also found another javadoc issue that was due to incorrect cut  paste when 
 the code was written) and ccc request
 
 As i mentioned in an earlier thread, these classes are hardly ever, if at 
 all used and would only be used when UDTs are used and the majority of 
 databases do not support this.
 
 
 Best
 lance
 On Jan 14, 2013, at 5:11 AM, Alan Bateman wrote:
 
 On 13/01/2013 23:51, Lance Andersen - Oracle wrote:
 :
 
 One other thing is that the CCE has a side-effect in that it consumes 
 the next attribute. The methods could be changed to peek at the next 
 attribute but that wouldn't work without synchronization or making it 
 clear in the spec that the it is not a thread-safe implementation of 
 SQLInput.
 I really want to keep the changes to the bare minimum here as this and the 
 other serial classes are hardly, if ever used at all.
 I understand, but if you add a catch-all in the class description to cover 
 the CCE case then this could be part of the same paragraph.
 
 -Alan
 
 
 Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
 Oracle Java Engineering
 1 Network Drive
 Burlington, MA 01803
 lance.ander...@oracle.com
 
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: review request 8006139, add missing methods to javax.sql.rowset.serial.SQLInput/OutputImpl

2013-01-19 Thread Lance Andersen - Oracle

On Jan 19, 2013, at 9:43 AM, Alan Bateman wrote:

 On 15/01/2013 16:48, Lance Andersen - Oracle wrote:
 Here is a revision
 
 http://cr.openjdk.java.net/~lancea/8006139/webrev.01
 
 I still have to enter the JBS entry for the javadoc clarifications (and I 
 also found another javadoc issue that was due to incorrect cut  paste when 
 the code was written) and ccc request
 
 As i mentioned in an earlier thread, these classes are hardly ever, if at 
 all used and would only be used when UDTs are used and the majority of 
 databases do not support this.
 
 It would be good to get the ClassCastException specified when you get time.

Yes will do once the ccc is approved on my list
 
 I looked at the latest webrev and it seems okay to me. 
 SQLInputImpl.getNextAttribute would be bit better if it only incremented idx 
 after retrieving an attribute but that is existing code. Minor intent issue 
 in SQLOutputImpl. writeNClob.
 
fixed the spacing.  going to leave idx alone for now as but will look at this 
for the future

Best
Lance
 -Alan.


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



  1   2   3   >