Code review request 6957230: CharsetEncoder.maxBytesPerChar() reports 4 for UTF-8; should be 3

2010-11-19 Thread Xueming Shen
Alan, Last time when Martin and I discussed this issue we agreed that the submitter is right about this. (The charset is a mapping between a sequence of bytes and a sequence of sisteen-bit Unicode characters, so the character discussed here should be a utf-16 character...)

Re: Code review request 6957230: CharsetEncoder.maxBytesPerChar() reports 4 for UTF-8; should be 3

2010-11-19 Thread Alan Bateman
Xueming Shen wrote: Alan, Last time when Martin and I discussed this issue we agreed that the submitter is right about this. (The charset is a mapping between a sequence of bytes and a sequence of sisteen-bit Unicode characters, so the character discussed here should be a utf-16

Re: Code review request 6631046: BufferedInputStream.available() reports negative int on very large inputstream

2010-11-19 Thread Alan Bateman
Mandy Chung wrote: 6631046: BufferedInputStream.available() reports negative int on very large inputstream Webrev at: http://cr.openjdk.java.net/~mchung/6631046/webrev.00/ InputStream.available() returns an int. For streams larger than 2GB, the FileInputStream.available() implementation

Re: Code review request 6858865: Fix for 6728376 causes regression if the size of data is 0 and malloc returns Null for 0-length

2010-11-19 Thread Alan Bateman
Xueming Shen wrote: Alan, It might not be a real regression if only consider the supported platforms (and yes, the malloc manpageI can found does clearly indicate NULL is for error). However I prefer to add some checks to make sure it behaves the same (compared to before the #6728376 change

Re: 6989471: compiler warnings building java/zip native code

2010-11-19 Thread Alan Bateman
Xueming Shen wrote: Alan, Kelly, Would you please help review the patch that tries to address those compiler warning in zip area? http://cr.openjdk.java.net/~sherman/6989471/webrev http://cr.openjdk.java.net/%7Esherman/6989471/webrev/ I added some comments to document the fact that

Re: Code review request 6402006: FileInputStream.available() returns negative values when reading a large file

2010-11-19 Thread Alan Bateman
Mike Duigou wrote: Would it be possible to call GetFileSizeEx() (or add a call to GetLastError()) MSDN: Note that if the return value is INVALID_FILE_SIZE (0x), an application must call GetLastError to determine whether the function has succeeded or failed. The reason the function

hg: jdk7/tl/jdk: 2 new changesets

2010-11-19 Thread michael . x . mcmahon
Changeset: 3092c842b0ea Author:michaelm Date: 2010-11-19 13:30 + URL: http://hg.openjdk.java.net/jdk7/tl/jdk/rev/3092c842b0ea 7001301: com/sun/net/httpserver/bugs/6725892/Test.java failing Reviewed-by: alanb ! test/com/sun/net/httpserver/bugs/6725892/Test.java Changeset:

Improved Java Collection APIs

2010-11-19 Thread John Platts
Here are improvements that I really want to see to Java Collection APIs: - Addition of an equality comparator interface. - An wrapper that wraps a java.util.Comparator as a equality comparator. - Map and set classes that allow an equality comparator to be used instead of the equals() method or

Re: Code review request 6402006: FileInputStream.available() returns negative values when reading a large file

2010-11-19 Thread Mandy Chung
On 11/19/10 2:11 AM, Alan Bateman wrote: I agree with Mike's suggestion to use GetFileSizeEx. Also, well spotted that the SetFilePointer usage is missing a check to GetLastError. I quickly checked the other usages of this clunky API and they seem to check the error. I'll send out a new

Re: Code review request 6858865: Fix for 6728376 causes regression if the size of data is 0 and malloc returns Null for 0-length

2010-11-19 Thread Xueming Shen
On 11/19/2010 01:31, Alan Bateman wrote: Xueming Shen wrote: Alan, It might not be a real regression if only consider the supported platforms (and yes, the malloc manpageI can found does clearly indicate NULL is for error). However I prefer to add some checks to make sure it behaves the

Re: Code review request 6957230: CharsetEncoder.maxBytesPerChar() reports 4 for UTF-8; should be 3

2010-11-19 Thread Ulf Zibis
IMO, you consequently should additionally correct the javadoc according the evaluation of bug 6957230. -Ulf Am 19.11.2010 08:55, schrieb Xueming Shen: Alan, Last time when Martin and I discussed this issue we agreed that the submitter is right about this. (The charset is a mapping between

Re: 6989471: compiler warnings building java/zip native code

2010-11-19 Thread Xueming Shen
Thanks Alan! webrev has been updated accordingly to fix the typo. -Sherman On 11/19/2010 01:38, Alan Bateman wrote: Xueming Shen wrote: Alan, Kelly, Would you please help review the patch that tries to address those compiler warning in zip area?

Re: Improved Java Collection APIs

2010-11-19 Thread Rémi Forax
Le 19/11/2010 16:49, John Platts a écrit : Here are improvements that I really want to see to Java Collection APIs: [...] - Addition of methods that iterates through the collection and invokes a callback. This is useful in cases where the callback object can be pre-allocated, since

Re: 6989471: compiler warnings building java/zip native code

2010-11-19 Thread Alan Bateman
Xueming Shen wrote: Thanks Alan! webrev has been updated accordingly to fix the typo. -Sherman Looks fine to me except the indentation at L1385 where it looks like you need to insert two additional spaces. -Alan.

Re: Code review request 6858865: Fix for 6728376 causes regression if the size of data is 0 and malloc returns Null for 0-length

2010-11-19 Thread Alan Bateman
Xueming Shen wrote: : We can probably do that for Inflater.c (and probably better do that at java level before we even come down here), but thing gets a little complicated for Deflater. One of the paths of the deflateBytes is to deal with parameter(s) change, so if malloc does not fail for

hg: jdk7/tl/jdk: 6631046: BufferedInputStream.available() reports negative int on very large inputstream

2010-11-19 Thread mandy . chung
Changeset: f30e1e1a4d90 Author:mchung Date: 2010-11-19 10:00 -0800 URL: http://hg.openjdk.java.net/jdk7/tl/jdk/rev/f30e1e1a4d90 6631046: BufferedInputStream.available() reports negative int on very large inputstream Reviewed-by: dholmes, alanb, mduigou !

Re: Please review java.util.jar.pack.* cleanup/refactoring/generificaiton

2010-11-19 Thread Kumar Srinivasan
Mike, Here is the new revision: http://cr.openjdk.java.net/~ksrini/6990106/webrev.03/ I nailed all the items per your suggestions. A few more I nits I noticed. BandStructure : allKQBands could be final. ClassReader : string switch opportunities in readAttributes() FixedArrayList : Seems to

Re: Code review request 6858865: Fix for 6728376 causes regression if the size of data is 0 and malloc returns Null for 0-length

2010-11-19 Thread Xueming Shen
On 11/19/2010 09:55 AM, Alan Bateman wrote: Xueming Shen wrote: : We can probably do that for Inflater.c (and probably better do that at java level before we even come down here), but thing gets a little complicated for Deflater. One of the paths of the deflateBytes is to deal with

hg: jdk7/tl/jdk: 2 new changesets

2010-11-19 Thread xueming . shen
Changeset: d9e4556acd4a Author:sherman Date: 2010-11-19 12:55 -0800 URL: http://hg.openjdk.java.net/jdk7/tl/jdk/rev/d9e4556acd4a 6989471: compiler warnings building java/zip native code Summary: remvoed the warning Reviewed-by: ohair, alanb !

Re: Code review request 6858865: Fix for 6728376 causes regression if the size of data is 0 and malloc returns Null for 0-length

2010-11-19 Thread Alan Bateman
Xueming Shen wrote: : I might not explain it clear enough. For Deflater.c we want to at least keep the malloc path for if ((*env)-GetBooleanField(env, this, setParamsID)) { } so it will continue go down to deflateParams() to reset the params, with a valid non-NULL ptr

Re: Code review request 6858865: Fix for 6728376 causes regression if the size of data is 0 and malloc returns Null for 0-length

2010-11-19 Thread Xueming Shen
On 11/19/2010 01:33 PM, Alan Bateman wrote: Xueming Shen wrote: : I might not explain it clear enough. For Deflater.c we want to at least keep the malloc path for if ((*env)-GetBooleanField(env, this, setParamsID)) { } so it will continue go down to deflateParams() to

Re: Code review request 6858865: Fix for 6728376 causes regression if the size of data is 0 and malloc returns Null for 0-length

2010-11-19 Thread Xueming Shen
On 11/19/2010 01:51 PM, Alan Bateman wrote: Xueming Shen wrote: : Why do you want to feed the zlib with a null buffer? :-) So are you saying that if the length is 0 that it still looks at the buffer? If so, then I'm okay with the last webrev. zlib does buffer null check before anything

hg: jdk7/tl/jdk: 2 new changesets

2010-11-19 Thread lance . andersen
Changeset: ff619988afac Author:lancea Date: 2010-11-19 17:15 -0500 URL: http://hg.openjdk.java.net/jdk7/tl/jdk/rev/ff619988afac 7000752: Duplicate entry in RowSetResourceBundles.properties Reviewed-by: alanb ! src/share/classes/com/sun/rowset/RowSetResourceBundle.properties !

Re: Improved Java Collection APIs

2010-11-19 Thread Dr Andrew John Hughes
On 19 November 2010 15:49, John Platts john_pla...@hotmail.com wrote: Here are improvements that I really want to see to Java Collection APIs: - Addition of an equality comparator interface. - An wrapper that wraps a java.util.Comparator as a equality comparator. - Map and set classes that

Re: Improved Java Collection APIs

2010-11-19 Thread David Holmes
John Platts said the following on 11/20/10 01:49: Here are improvements that I really want to see to Java Collection APIs: - Additional concurrent collection classes. Some of these classes will require synchronized modification, while supporting thread-safe unsynchronized access and concurrent

Re: Code review request 6858865: Fix for 6728376 causes regression if the size of data is 0 and malloc returns Null for 0-length

2010-11-19 Thread Xueming Shen
Alan, After staring those simple, 11 lines of change for minutes, I believe we should simply go back with the original approach at http://cr.openjdk.java.net/~sherman/6858865/webrev.00 The change in http://cr.openjdk.java.net/~sherman/6858865/webrev.00 obviously is problematic, especially

hg: jdk7/tl/jdk: 2 new changesets

2010-11-19 Thread valerie . peng
Changeset: 6deeca9378c0 Author:valeriep Date: 2010-11-19 16:59 -0800 URL: http://hg.openjdk.java.net/jdk7/tl/jdk/rev/6deeca9378c0 6203816: Can not run test/java/security/Security/ClassLoaderDeadlock.sh from the command line Summary: Fixed the script to not delete the provider

6613829: (docs) Readable.read() ReadOnlyBufferException is not linked

2010-11-19 Thread Alan Bateman
I need a reviewer to a trivial drive-by fix to java.lang.Readable's javadoc. Thanks, Alan. diff --git a/src/share/classes/java/lang/Readable.java b/src/share/classes/java/lang/Readable.java --- a/src/share/classes/java/lang/Readable.java +++ b/src/share/classes/java/lang/Readable.java @@