Code review request 6957230: CharsetEncoder.maxBytesPerChar() reports 4 for UTF-8; should be 3
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...) http://cr.openjdk.java.net/~sherman/6957230/webrev Thanks, -Sherman
Re: Code review request 6957230: CharsetEncoder.maxBytesPerChar() reports 4 for UTF-8; should be 3
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 character...) http://cr.openjdk.java.net/~sherman/6957230/webrev Thanks, -Sherman I'll go along with this too, so thumbs up from me. -Alan.
Re: Code review request 6631046: BufferedInputStream.available() reports negative int on very large inputstream
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 returns Integer.MAX_VALUE which is a reasonable choice. java.io.BufferedInputStream and java.io.PushbackInputStream maintain a buffer. The available() method in these 2 classes returns the sum of the number of bytes remaining to be read in the buffer and the result of calling the in.available(). If in.available() returns a large number, the result may overflow and a negative size will be returned. This fixes to handle the overflow case properly. Thanks Mandy Looks good to me too - thanks for taking this one. -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
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 went it), even on the weird OS that Mario has. Anyway, a 0-length really malloc should not trigger a OOME. http://cr.openjdk.java.net/~sherman/6858865/webrev The webrev for #6728376 is at http://cr.openjdk.java.net/~sherman/6728376/webrev Thanks, -Sherman I think this one has come up before [1]. Looking at it now, I wonder if it would be simpe for inflate to just return 0 if the input buffer or the max number of uncompressed bytes is 0. That is, just don't attempt the mallocs for these cases. -Alan [1] http://mail.openjdk.java.net/pipermail/core-libs-dev/2009-July/002028.html
Re: 6989471: compiler warnings building java/zip native code
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 ZIP_Read/FindEntry/InflateFully can't deal with 2**32 byte (the only user of these native method is the vm, I don't think they are trying to de-compress a 4G entry in one invocation any time soon). We might find interesting to support 2**32 entry for those methods, but obviously it is not the purpose of this bug. Same for the dstLen of the compress/uncompress methods, the original zlib interface has the limitation of 2**32. I only tried to remove the warning in this patch. I'm not dealing with the POSIX strdup warning. It appears Kumar has already address the warning in zcrc32.c. Thanks, Sherman In zip_util.c there is a comment that reads reading A entry. Is that a typo that should be reading an entry. Also looks like an indent problem at L1377. Otherwise looks fine to me. -Alan.
Re: Code review request 6402006: FileInputStream.available() returns negative values when reading a large file
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 may appear to fail when it has not is that lpFileSizeHigh could be non-NULL or the file size could be 0x. In this case, GetLastError will return NO_ERROR (0) upon success. Because of this behavior, it is recommended that you use GetFileSizeEx instead. There is a similar problem for for SetFilePosition(): MSDN: Note Because INVALID_SET_FILE_POINTER is a valid value for the low-order DWORD of the new file pointer, you must check both the return value of the function and the error code returned by GetLastError to determine whether or not an error has occurred. If an error has occurred, the return value of SetFilePointer is INVALID_SET_FILE_POINTER and GetLastError returns a value other than NO_ERROR. For a code example that demonstrates how to check for failure, see the Remarks section in this topic. Mike 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. Do you plan to include a regression test for this? I know it's not nice for tests to be create multi-GB files but this is one that we should have caught a long time ago. If we create it as a sparse file then the test should be relatively quick. -Alan.
hg: jdk7/tl/jdk: 2 new changesets
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: 892c54251ac8 Author:michaelm Date: 2010-11-19 13:35 + URL: http://hg.openjdk.java.net/jdk7/tl/jdk/rev/892c54251ac8 Merge
Improved Java Collection APIs
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 identity comparisons - New map and set interfaces that behave like the existing java.util.Map and java.util.Set interfaces. Unlike the contract of the java.util.Map and java.util.Set interfaces, the contract of the new map and set interfaces will not require that o1 be considered to be equal to o2 when o1 != o2 and o1.equals(o2) are both true. The new map and set interfaces will still require that o1 be considered equal to o2 whenever o1 == o2 is true. - New collection interfaces which define versions of search and removal operations that take an additional equality comparator argument. This enables search and removal operations to use an equality comparator to determine equality instead of relying on the equals method, an identity comparison, or the java.util.Comparator.compare method. - Additional concurrent collection classes. Some of these classes will require synchronized modification, while supporting thread-safe unsynchronized access and concurrent iteration. - Support for collections synchronized on a read-write lock. - 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 iteration using a java.util.Iterator object usually requires an iterator to be allocated each time an iteration takes place.
Re: Code review request 6402006: FileInputStream.available() returns negative values when reading a large file
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 webrev. Do you plan to include a regression test for this? I know it's not nice for tests to be create multi-GB files but this is one that we should have caught a long time ago. If we create it as a sparse file then the test should be relatively quick. I have a regression test but didn't plan to include in the changeset. I'll modify it to create a sparse file as you suggest and include it in the webrev. Thanks Mandy
Re: Code review request 6858865: Fix for 6728376 causes regression if the size of data is 0 and malloc returns Null for 0-length
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 same (compared to before the #6728376 change went it), even on the weird OS that Mario has. Anyway, a 0-length really malloc should not trigger a OOME. http://cr.openjdk.java.net/~sherman/6858865/webrev The webrev for #6728376 is at http://cr.openjdk.java.net/~sherman/6728376/webrev Thanks, -Sherman I think this one has come up before [1]. Looking at it now, I wonder if it would be simpe for inflate to just return 0 if the input buffer or the max number of uncompressed bytes is 0. That is, just don't attempt the mallocs for these cases. -Alan [1] http://mail.openjdk.java.net/pipermail/core-libs-dev/2009-July/002028.html 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 0-length request (true on all supported platforms), the deflateBytes goes down to zlib's deflateParams() and if there is nothing left to flush, this invocation can successfully re-set the param(s) and return 0. The reason why I decided to go with the safe and simple solution last night, the proposed webrev at least should behave the exactly the same as it was before. -Sherman
Re: Code review request 6957230: CharsetEncoder.maxBytesPerChar() reports 4 for UTF-8; should be 3
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 a sequence of bytes and a sequence of sisteen-bit Unicode characters, so the character discussed here should be a utf-16 character...) http://cr.openjdk.java.net/~sherman/6957230/webrev Thanks, -Sherman
Re: 6989471: compiler warnings building java/zip native code
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? 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 ZIP_Read/FindEntry/InflateFully can't deal with 2**32 byte (the only user of these native method is the vm, I don't think they are trying to de-compress a 4G entry in one invocation any time soon). We might find interesting to support 2**32 entry for those methods, but obviously it is not the purpose of this bug. Same for the dstLen of the compress/uncompress methods, the original zlib interface has the limitation of 2**32. I only tried to remove the warning in this patch. I'm not dealing with the POSIX strdup warning. It appears Kumar has already address the warning in zcrc32.c. Thanks, Sherman In zip_util.c there is a comment that reads reading A entry. Is that a typo that should be reading an entry. Also looks like an indent problem at L1377. Otherwise looks fine to me. -Alan.
Re: Improved Java Collection APIs
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 iteration using a java.util.Iterator object usually requires an iterator to be allocated each time an iteration takes place. This one is on the task list of the lambda JSR, the callback will be expressed as a lambda. Rémi
Re: 6989471: compiler warnings building java/zip native code
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
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 0-length request (true on all supported platforms), the deflateBytes goes down to zlib's deflateParams() and if there is nothing left to flush, this invocation can successfully re-set the param(s) and return 0. The reason why I decided to go with the safe and simple solution last night, the proposed webrev at least should behave the exactly the same as it was before. -Sherman I think it would be a bit cleaner for inflater. For deflater would it cleaner to just skip the mallocs when the lengths are 0 (for the deflateParams path). -Alan.
hg: jdk7/tl/jdk: 6631046: BufferedInputStream.available() reports negative int on very large inputstream
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 ! src/share/classes/java/io/BufferedInputStream.java ! src/share/classes/java/io/PushbackInputStream.java
Re: Please review java.util.jar.pack.* cleanup/refactoring/generificaiton
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 be unchanged since last webrev. Was nothing saved by extending AbstractList? Initially I was extending AbstractList and later changed it to implement List. And no I did not see any change. Thanks Kumar ConstantPool : equals() only works as long as all sub-classes have different tag values. (see sameTagAs() which uses instanceof rather than getClass()). Using getClass() likely makes tag comparison unnecessary. Package : another InnerClass.equals() using instanceof PackerImpl : several uses of XXX.size() 0 which could be !XXX.isEmpty() which is generally preferred as size() is not O(1) for some collections where isEmpty() is O(1). PropMap : could _listeners and theMap be private? Since initialization of theMap doesn't depend upon any constructor params why not construct it at declaration? Mike On Nov 18 2010, at 14:58 , Kumar Srinivasan wrote: Hi Mike, Thanks for the review, here is the new version: http://cr.openjdk.java.net/~ksrini/6990106/webrev.02/
Re: Code review request 6858865: Fix for 6728376 causes regression if the size of data is 0 and malloc returns Null for 0-length
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 parameter(s) change, so if
malloc does not
fail for 0-length request (true on all supported platforms), the
deflateBytes goes down
to zlib's deflateParams() and if there is nothing left to flush,
this invocation can successfully
re-set the param(s) and return 0. The reason why I decided to go with
the safe and simple
solution last night, the proposed webrev at least should behave the
exactly the same as it
was before.
-Sherman
I think it would be a bit cleaner for inflater. For deflater would it
cleaner to just skip the mallocs when the lengths are 0 (for the
deflateParams path).
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 on most platforms, even for 0-length buf, this is
what it was before #6728376 and what it is now, We don't what to have
another regression (change the params, invoke the deflate() with 0-length
buffer, but now it returns 0 with change the setting, I have to invoke it
again with...) here. And with the len != 0 check, it will be no behavior
change as well for that weird OS.
For other 2 paths (the Inflater.c and the
(*env)-GetBooleanField(env, this, setParamsID) == false
path, I believe/think/guess it should be safe to bypath the zlib by
checking len/in_len and returning 0 if any of them is zero. But this is
again a change, probably:-) harmless giving my understand/reading of
zlib code and the existing regression tests.
So the bit cleaner webrev is
http://cr.openjdk.java.net/~sherman/6858865/webrev/
But it deal with the 0-length issue differently at different places.
This is why I proposed last night the safe and simple: change.
Anyway, I'm fine with the bit-cleaner approach, so if you're OK with
it, throw it in:-)
The old webrev is at
http://cr.openjdk.java.net/~sherman/6858865/webrev.00/
-Sherman
hg: jdk7/tl/jdk: 2 new changesets
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 ! src/share/native/java/util/zip/zip_util.c ! src/share/native/java/util/zip/zlib-1.2.3/compress.c ! src/share/native/java/util/zip/zlib-1.2.3/uncompr.c Changeset: b44704ce8a08 Author:sherman Date: 2010-11-19 12:58 -0800 URL: http://hg.openjdk.java.net/jdk7/tl/jdk/rev/b44704ce8a08 6957230: CharsetEncoder.maxBytesPerChar() reports 4 for UTF-8; should be 3 Summary: changged utf-8's CharsetEncoder.maxBytesPerChar to 3 Reviewed-by: alanb ! src/share/classes/sun/nio/cs/UTF_8.java
Re: Code review request 6858865: Fix for 6728376 causes regression if the size of data is 0 and malloc returns Null for 0-length
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 on most platforms, even for 0-length buf, this is
what it was before #6728376 and what it is now, We don't what to have
another regression (change the params, invoke the deflate() with
0-length
buffer, but now it returns 0 with change the setting, I have to invoke it
again with...) here. And with the len != 0 check, it will be no
behavior
change as well for that weird OS.
For other 2 paths (the Inflater.c and the
(*env)-GetBooleanField(env, this, setParamsID) == false
path, I believe/think/guess it should be safe to bypath the zlib by
checking len/in_len and returning 0 if any of them is zero. But this is
again a change, probably:-) harmless giving my understand/reading of
zlib code and the existing regression tests.
So the bit cleaner webrev is
http://cr.openjdk.java.net/~sherman/6858865/webrev/
But it deal with the 0-length issue differently at different places.
This is why I proposed last night the safe and simple: change.
Anyway, I'm fine with the bit-cleaner approach, so if you're OK with
it, throw it in:-)
The old webrev is at
http://cr.openjdk.java.net/~sherman/6858865/webrev.00/
-Sherman
The updated webrev looks better, but for the deflateParam path then
maybe you could do something like this:
if (this_len == 0) {
in_buf = NULL;
} else {
in_buf = (jbyte*) malloc(this_len);
if (in_buf == NULL) {
JNU_ThrowOutOfMemoryError(env, 0);
return 0;
}
}
and same thing for out_buf.
Would that be a bit cleaner?
-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
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 reset the params, with
a valid non-NULL ptr on most platforms, even for 0-length buf, this is
what it was before #6728376 and what it is now, We don't what to have
another regression (change the params, invoke the deflate() with
0-length
buffer, but now it returns 0 with change the setting, I have to
invoke it
again with...) here. And with the len != 0 check, it will be no
behavior
change as well for that weird OS.
For other 2 paths (the Inflater.c and the
(*env)-GetBooleanField(env, this, setParamsID) == false
path, I believe/think/guess it should be safe to bypath the zlib by
checking len/in_len and returning 0 if any of them is zero. But this is
again a change, probably:-) harmless giving my understand/reading of
zlib code and the existing regression tests.
So the bit cleaner webrev is
http://cr.openjdk.java.net/~sherman/6858865/webrev/
But it deal with the 0-length issue differently at different places.
This is why I proposed last night the safe and simple: change.
Anyway, I'm fine with the bit-cleaner approach, so if you're OK with
it, throw it in:-)
The old webrev is at
http://cr.openjdk.java.net/~sherman/6858865/webrev.00/
-Sherman
The updated webrev looks better, but for the deflateParam path then
maybe you could do something like this:
if (this_len == 0) {
in_buf = NULL;
} else {
in_buf = (jbyte*) malloc(this_len);
if (in_buf == NULL) {
JNU_ThrowOutOfMemoryError(env, 0);
return 0;
}
}
and same thing for out_buf.
Would that be a bit cleaner?
Why do you want to feed the zlib with a null buffer? :-)
The zlib#deflate() does some null checks at the beginning as showed below.
I probably can do something tricky to make it work, but for me I don't see
the benefit of taking the risk. If it works don't break it:-)
-Sherman
int ZEXPORT deflate (strm, flush)
z_streamp strm;
int flush;
{
int old_flush; /* value of flush param for previous deflate call */
deflate_state *s;
if (strm == Z_NULL || strm-state == Z_NULL ||
flush Z_FINISH || flush 0) {
return Z_STREAM_ERROR;
}
s = strm-state;
if (strm-next_out == Z_NULL ||
(strm-next_in == Z_NULL strm-avail_in != 0) ||
(s-status == FINISH_STATE flush != Z_FINISH)) {
ERR_RETURN(strm, Z_STREAM_ERROR);
}
if (strm-avail_out == 0) ERR_RETURN(strm, Z_BUF_ERROR);
-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
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 else, just like we do null check/throw NPE first then check the offset and length. -Sherman
hg: jdk7/tl/jdk: 2 new changesets
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 ! src/share/classes/com/sun/rowset/internal/XmlReaderContentHandler.java Changeset: bf407ff3e97b Author:lancea Date: 2010-11-19 17:18 -0500 URL: http://hg.openjdk.java.net/jdk7/tl/jdk/rev/bf407ff3e97b 7001669: Typo in javadocs for SQLPermission Reviewed-by: alanb ! src/share/classes/java/sql/SQLPermission.java
Re: Improved Java Collection APIs
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 allow an equality comparator to be used instead of the equals() method or identity comparisons - New map and set interfaces that behave like the existing java.util.Map and java.util.Set interfaces. Unlike the contract of the java.util.Map and java.util.Set interfaces, the contract of the new map and set interfaces will not require that o1 be considered to be equal to o2 when o1 != o2 and o1.equals(o2) are both true. The new map and set interfaces will still require that o1 be considered equal to o2 whenever o1 == o2 is true. - New collection interfaces which define versions of search and removal operations that take an additional equality comparator argument. This enables search and removal operations to use an equality comparator to determine equality instead of relying on the equals method, an identity comparison, or the java.util.Comparator.compare method. - Additional concurrent collection classes. Some of these classes will require synchronized modification, while supporting thread-safe unsynchronized access and concurrent iteration. - Support for collections synchronized on a read-write lock. - 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 iteration using a java.util.Iterator object usually requires an iterator to be allocated each time an iteration takes place. Feel free to post patches to add these features. Then they can be reviewed and maybe added to the JDK. -- Andrew :-) Free Java Software Engineer Red Hat, Inc. (http://www.redhat.com) Support Free Java! Contribute to GNU Classpath and the OpenJDK http://www.gnu.org/software/classpath http://openjdk.java.net PGP Key: 94EFD9D8 (http://subkeys.pgp.net) Fingerprint: F8EF F1EA 401E 2E60 15FA 7927 142C 2591 94EF D9D8
Re: Improved Java Collection APIs
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 iteration. That's a somewhat vague request. If you have specific ideas/suggestions here feel free to raise them on the concurrency-inter...@cs.oswego.edu mailing list where Doug Lea and the JSR-166 EG will see them (along with a large chunk of the community interested in concurrent collections). - Support for collections synchronized on a read-write lock. Again can you be more specific? Read-write locks sound theoretically useful for some collection operations but the devil is in the details. Unlike synchronized collections you can't necessarily just use a wrapper. David Holmes
Re: Code review request 6858865: Fix for 6728376 causes regression if the size of data is 0 and malloc returns Null for 0-length
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 in case like in_len == 0 but out_len != 0 (no more, or no more new input, but with a valid output buffer), it's definitely possible the inflater/deflater might have more bites to (and should) output in this scenario, it's a bug to return 0 here. Sure it's possible to go further like if (in_len == 0) return 0; if (len == 0) ... But given the purpose of this fix is to solve that particular regression (which actually does not cause any regression for mainstream platforms), I prefer not take the risk of causing another real regression/behavior change here, especially we got burned couple times here in the past when tried to do better. Agree? -Sherman On 11/19/2010 02:03 PM, Xueming Shen wrote: 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 else, just like we do null check/throw NPE first then check the offset and length. -Sherman
hg: jdk7/tl/jdk: 2 new changesets
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 sub-directory Reviewed-by: weijun ! test/java/security/Security/ClassLoaderDeadlock/ClassLoaderDeadlock.sh ! test/java/security/Security/ClassLoaderDeadlock/Deadlock2.sh Changeset: 784f2f094051 Author:valeriep Date: 2010-11-19 17:05 -0800 URL: http://hg.openjdk.java.net/jdk7/tl/jdk/rev/784f2f094051 6720456: New 4150 may have larger blowfish keysizes Summary: Changed to use TBD value instead of FAIL Reviewed-by: weijun ! test/sun/security/pkcs11/KeyGenerator/TestKeyGenerator.java
6613829: (docs) Readable.read() ReadOnlyBufferException is not linked
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
@@ -44,11 +44,11 @@ public interface Readable {
* rewinding of the buffer is performed.
*
* @param cb the buffer to read characters into
- * @return @return The number of ttchar/tt values added to the
buffer,
+ * @return The number of {...@code char} values added to the buffer,
* or -1 if this source of characters is at its end
* @throws IOException if an I/O error occurs
* @throws NullPointerException if cb is null
- * @throws ReadOnlyBufferException if cb is a read only buffer
+ * @throws java.nio.ReadOnlyBufferException if cb is a read only buffer
*/
public int read(java.nio.CharBuffer cb) throws IOException;
