Re: Please Review Separate temporal interface layer (8025720)

2013-10-15 Thread Xueming Shen
looks good. On 10/14/2013 10:49 AM, roger riggs wrote: Hi, The concrete argument types of Chronology and ZoneId in the TemporalField.resolve method are too specific and should be generalized to allow greater access to the Temporal being resolved. They should be replaced by a single more

RFR 8025971 and 8026197

2013-10-15 Thread Xueming Shen
Hi, Please help codereview the changes for 8025971: Remove Time-Zone IDs HST/EST/MST 8026197: Slow reading tzdb.dat if the JRE is on a high-latency, remote file system http://cr.openjdk.java.net/~sherman/8025971_8026197/webrev Note: for 8026197, the only real native resource here needed to

Re: RFR 8025971 and 8026197

2013-10-15 Thread Xueming Shen
3:35 PM, Xueming Shen wrote: Hi, Please help codereview the changes for 8025971: Remove Time-Zone IDs HST/EST/MST 8026197: Slow reading tzdb.dat if the JRE is on a high-latency, remote file system http://cr.openjdk.java.net/~sherman/8025971_8026197/webrev Note: for 8026197, the only real

Re: RFR [8023431] Test java/util/zip/GZIP/GZIPInZip.java failed

2013-10-14 Thread Xueming Shen
looks fine. On 10/14/2013 02:51 AM, Ivan Gerasimov wrote: Hello all! Would you please help review a fix for the intermittently failing regtest? http://bugs.sun.com/view_bug.do?bug_id=8023431 The reasons of the failures were non closed PipedInput/OutputStreams. Thus Read/write dead end were

Re: Deflater enhancements

2013-10-08 Thread Xueming Shen
The 100 in sample really means a big enough buffer here for simple use case, not necessarily means a fixed size' buffer. Sure there is always room for doc improvement, especially given this is the API has been there for decade. Deflater/Inflater.end() is for explicitly/proactively release of

Re: Review Request for project Threeten updates

2013-10-04 Thread Xueming Shen
== false - Clarified in Temporal.until that the *converted* input temporal is passed to the between method. http://cr.openjdk.java.net/~rriggs/webrev-period-until-8023762-807-834-835/ Thanks, Roger On 10/4/2013 6:16 AM, Stephen Colebourne wrote: On 3 October 2013 23:08, Xueming Shen

Re: Review java.time test refactoring

2013-10-03 Thread Xueming Shen
Looks fine. -Sherman On 10/01/2013 11:53 AM, roger riggs wrote: Ping, needs a Reviewer. http://cr.openjdk.java.net/~rriggs/webrev-serial-refactor-8024896/ Updated the webrev with the renamed test classes to distinguish them from the non-serialization tests in a different package. Thanks,

Re: Review Request for project Threeten updates

2013-10-03 Thread Xueming Shen
(1) until(Temporal endExclusive, TEmporalUnit unit) - Shouldn't we invoke requireNonNull() for unit before invoking unit.between(...)? (2) It appears we started to use endExclusive in until() methods, while the param has been renamed to be exclusive explicitly, personally I prefer

Re: Please Review fix for reduced value parser 8024076

2013-10-02 Thread Xueming Shen
Should move the static field BASE_DATE into ReducePrinterParser? Logically (and for performance, if it matters at all) RPP appears to be a better place for this constant. The rest looks fine. -Sherman On 10/02/2013 08:19 AM, roger riggs wrote: Please review this fix for parsing two digit

Re: Please Review fix for reduced value parser 8024076

2013-10-02 Thread Xueming Shen
implementation, so I don't see any probably logically to move it into RPP, maybe rename it to ISO_BASE_DATE... -Sherman Roger On 10/2/2013 12:54 PM, Xueming Shen wrote: Should move the static field BASE_DATE into ReducePrinterParser? Logically (and for performance, if it matters at all) RPP appears

Re: Please Review java.time static interface methods move to supporting class

2013-10-02 Thread Xueming Shen
Hi, While it might make sense to have method to return the adjuster for TemporalAdjusters, is there any real benefit to have those parentheses for TemporalQueries? Maybe it can just be an enum? Or we are considering adding more parametrized adjusters in the future? Aren't we? Now all interfaces

hg: jdk8/tl/jdk: 8023113: tools/jar/ChangeDir.java fails if /tmp/a exists

2013-09-19 Thread xueming . shen
Changeset: 278873b2b3f8 Author:sherman Date: 2013-09-19 10:06 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/278873b2b3f8 8023113: tools/jar/ChangeDir.java fails if /tmp/a exists Summary: updated the test case Reviewed-by: alanb ! test/tools/jar/ChangeDir.java

Re: RFR: 8023113: tools/jar/ChangeDir.java fails if /tmp/a exists

2013-09-19 Thread Xueming Shen
On 9/19/2013 1:52 AM, Alan Bateman wrote: On 18/09/2013 20:45, Xueming Shen wrote: OK, how about this one? I'm even using lambda:-) http://cr.openjdk.java.net/~sherman/8023113/webrev/ -Sherman Looks okay except that Files.list returns a stream that needs to be closed to avoid leaking

RFR: 6233323,,ZipEntry.isDirectory() may return false incorrectly

2013-09-19 Thread Xueming Shen
Hi, Please help review the change for #6233323. http://cr.openjdk.java.net/~sherman/6233323/webrev Background info: ZipFile.getEntry() is implemented the way that it first tries to find the entry with the exactly matched name. If failed, it then tries to see if the specified name ends with a

Re: RFR: 8023113: tools/jar/ChangeDir.java fails if /tmp/a exists

2013-09-18 Thread Xueming Shen
OK, how about this one? I'm even using lambda:-) http://cr.openjdk.java.net/~sherman/8023113/webrev/ -Sherman On 09/18/2013 09:42 AM, Xueming Shen wrote: On 09/18/2013 03:31 AM, Alan Bateman wrote: On 17/09/2013 22:55, Xueming Shen wrote: Hi, Please help the small update to the test case

RFR: 8023113: tools/jar/ChangeDir.java fails if /tmp/a exists

2013-09-17 Thread Xueming Shen
Hi, Please help the small update to the test case tools/jar/ChangeDir.java, which might fail if the dedicated test directory /tmp/a exists and the user does not have privilege to delete/clean it up. The proposed change here is to use a random file name (from File.getTempFile()) for the top test

hg: jdk8/tl/jdk: 7186311: (props) Unicode is misspelled as Uniocde in JavaDoc and error message

2013-09-15 Thread xueming . shen
Changeset: b9d59414de23 Author:sherman Date: 2013-09-15 11:16 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/b9d59414de23 7186311: (props) Unicode is misspelled as Uniocde in JavaDoc and error message Summary: to correct the typo Reviewed-by: alanb, chegar !

Re: RFR: 8920687: Deflater.setLevel does not work as expected

2013-09-15 Thread Xueming Shen
in the buffer or not. So I think it might be good to set the expectation clear here. http://cr.openjdk.java.net/~sherman/8020687/webrev/ -Sherman On 9/14/13 5:24 AM, Alan Bateman wrote: On 13/09/2013 20:44, Xueming Shen wrote: Hi, This is change to clarify the java doc to match the existing behavior

Re: RFR: 8920687: Deflater.setLevel does not work as expected

2013-09-15 Thread Xueming Shen
Updated accordingly. http://cr.openjdk.java.net/~sherman/8020687/webrev/ Thanks! -Sherman On 9/15/13 12:45 PM, Alan Bateman wrote: On 15/09/2013 19:30, Xueming Shen wrote: Thanks Alan. I dropped the current in setLevel. Yes, I would like to be explicit about the deflate invocation here

hg: jdk8/tl/jdk: 8020687: Deflater.setLevel does not work as expected

2013-09-15 Thread xueming . shen
Changeset: efa09bf27d39 Author:sherman Date: 2013-09-15 13:58 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/efa09bf27d39 8020687: Deflater.setLevel does not work as expected Summary: to clarify the api to match the existing implementation behavior Reviewed-by: alanb !

RFR: 7186311: (props) Unicode is misspelled as Uniocde in JavaDoc and error message

2013-09-14 Thread Xueming Shen
Hi, Please help review this trivial javadoc typo fix. http://cr.openjdk.java.net/~sherman/7186311/webrev Thanks, -Sherman

RFR: 8920687: Deflater.setLevel does not work as expected

2013-09-13 Thread Xueming Shen
Hi, This is change to clarify the java doc to match the existing behavior. If there is a pending level/strategy change (via setLevel/Stragety()) when deflate(...) is invoked, the implementation goes down to zlib's deflateParams() for deflating operation, which clearly specifies its behavior

Re: Please review two corrections for java.time

2013-09-13 Thread Xueming Shen
looks fine. On 9/13/13 12:07 PM, roger riggs wrote: Ping, Reviewer needed for these minor updates, the test is now more robust thanks to Peter's nudging. http://cr.openjdk.java.net/~rriggs/webrev-localtime-now-8023639/ Please review for two corrections: - The

RFR: 8024338: Constant fields introduced by JDK-4759491 fix in b94 are exposed as public fields in public API

2013-09-11 Thread Xueming Shen
Hi, The background of this bug is the anti-pattern of defining constants in interfaces. In this case all the constants defined in package private interface ZipConstants are being exposed as public fields of the classes that implements it. This is JDK 1.0 era design/implementation, something

hg: jdk8/tl/jdk: 8024338: Constant fields introduced by JDK-4759491 fix in b94 are exposed as public fields in public API

2013-09-11 Thread xueming . shen
Changeset: 60b4cbdb446d Author:sherman Date: 2013-09-11 11:29 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/60b4cbdb446d 8024338: Constant fields introduced by JDK-4759491 fix in b94 are exposed as public fields in public API Summary: to move the new constants out of

RFR: Faster ZipFile.getEntry()/entries()

2013-09-05 Thread Xueming Shen
Hi, The change proposed here is to bring the zip entry handing code from the native level to the java level. This effectively solves the performance issues of ZipFile.getEntry and entries() that is caused by multiple jni invocation steps to generate one single ZipEntry (see

hg: jdk8/tl/jdk: 6341345: (spec) Console.reader() should make it clear that the reader requires line termination

2013-09-04 Thread xueming . shen
Changeset: 478afc30679b Author:sherman Date: 2013-09-04 12:35 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/478afc30679b 6341345: (spec) Console.reader() should make it clear that the reader requires line termination Summary: to clarify the spec Reviewed-by: alanb !

hg: jdk8/tl/jdk: 7186632: NLS t13y issue on jar.properties file

2013-09-04 Thread xueming . shen
Changeset: c6a4df06d57e Author:sherman Date: 2013-09-04 12:37 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/c6a4df06d57e 7186632: NLS t13y issue on jar.properties file Summary: to remove the redundant backslash Reviewed-by: naoto !

Re: RFR: JDK-6341345: (spec) Console.reader() should make it clear that the reader requires line termination

2013-08-29 Thread Xueming Shen
On 08/29/2013 12:03 AM, Alan Bateman wrote: On 28/08/2013 22:28, Xueming Shen wrote: Alan, My mailbox suggests that you have already reviewed this one back to the past May, somehow I dropped the ball some where. Here is the webrev and CCC again (the change is a trivial spec clarification

RFR: JDK-7186632: NLS t13y issue on jar.properties file

2013-08-29 Thread Xueming Shen
Hi, Please help review the change for #7186632. http://cr.openjdk.java.net/~sherman/7186632/webrev The proposed change here is to remove the backslash before the word inflated:. While based on j.u.Properties spec it is absolutely legal and harmless to have a backslash before a non-valid escape

Re: RFR JDK-8023713: ZipFileSystem has compatiable issue to handle old zip file.

2013-08-28 Thread Xueming Shen
On 08/28/2013 03:56 AM, Alan Bateman wrote: On 28/08/2013 00:09, Xueming Shen wrote: On 08/27/2013 03:07 PM, Martin Buchholz wrote: It does seem vaguely reasonable to support any extra data. Don't you want to also handle arbitrary byte arrays, if e.g. one the 16-bit size fields overflows

hg: jdk8/tl/jdk: 8023713: ZipFileSystem crashes on old zip file

2013-08-28 Thread xueming . shen
Changeset: 690b2931baef Author:sherman Date: 2013-08-28 09:46 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/690b2931baef 8023713: ZipFileSystem crashes on old zip file Summary: to handle extra data field copy correctly even the extra data does not follow the spec

Re: RFR JDK-8023713: ZipFileSystem has compatiable issue to handle old zip file.

2013-08-28 Thread Xueming Shen
to me. On Tue, Aug 27, 2013 at 4:09 PM, Xueming Shen xueming.s...@oracle.com mailto:xueming.s...@oracle.com wrote: On 08/27/2013 03:07 PM, Martin Buchholz wrote: It does seem vaguely reasonable to support any extra data. Don't you want to also handle arbitrary byte arrays, if e.g

RFR: JDK-6341345: (spec) Console.reader() should make it clear that the reader requires line termination

2013-08-28 Thread Xueming Shen
Alan, My mailbox suggests that you have already reviewed this one back to the past May, somehow I dropped the ball some where. Here is the webrev and CCC again (the change is a trivial spec clarification, which I should have done 8 years ago). http://cr.openjdk.java.net/~sherman/6341345

hg: jdk8/tl/jdk: 8023647: abc1c.matches((\\w)+1\\1)) returns false

2013-08-27 Thread xueming . shen
Changeset: 3f6777cbfe69 Author:sherman Date: 2013-08-27 12:54 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/3f6777cbfe69 8023647: abc1c.matches((\\w)+1\\1)) returns false Summary: to correct the wrong GroupCurly group index backoff code Reviewed-by: alanb !

RFR JDK-8023713: ZipFileSystem has compatiable issue to handle old zip file.

2013-08-27 Thread Xueming Shen
Hi, Please help review the change for #8023713 http://cr.openjdk.java.net/~sherman/8023713/webrev The root cause is that the newly introduced ZOS.writeExtra() (for #8015666) fails to handle irregular extra data field. The zip spec requires the the extra data stars with 4 bytes of tag + size

Re: RFR JDK-8023713: ZipFileSystem has compatiable issue to handle old zip file.

2013-08-27 Thread Xueming Shen
. probably I should. The webrev has been updated to simply copy the rest of the extra data, if the sz is either 0 or out of the range. http://cr.openjdk.java.net/~sherman/8023713/webrev/ And put a SPACE after if. updated. Thanks! -Sherman On Tue, Aug 27, 2013 at 2:42 PM, Xueming Shen xueming.s

Re: RFR 8022761: SQE test regression on wrongly signed indexed jar file

2013-08-26 Thread Xueming Shen
On 08/19/2013 06:11 AM, Weijun Wang wrote: Hi Sherman I try out jar i after signing and it puts INDEX.LIST at the very beginning of the file. Does this mean INDEX.LIST was actually an exception? Or it's just a bug? Anyway, I think I should update the fix for 8021788 and here is the webrev:

RFR: JDK-8023647,, abc1c .matches( (\\w)+1\\1 )) returns false

2013-08-26 Thread Xueming Shen
Hi, Please help review the proposed change for 8023647: http://cr.openjdk.java.net/~sherman/8023647/webrev It appears group index updating in situation of GroupCurly backing off is incorrect from day one (the bug is reproducible back to jdk5). The current implementation does not back off the

Re: Please review Meiji era tidyup

2013-08-16 Thread Xueming Shen
Looks good. -Sherman On 08/14/2013 12:48 PM, roger riggs wrote: The issue 8019185: Inconsistency between JapaneseEra start dates and java.util.JapaneseImperialDate Observes a minor discrepancy between the java.util.Calendar Japanese implementation and the java.time JapaneseEra. This fix

Re: RFR: JDK-7154662: {CRC32, Adler32}.update(byte[] b, int off, int len): undocumented ArrayIndexOutOfBoundsException

2013-08-15 Thread Xueming Shen
On 08/15/2013 05:15 AM, Florian Weimer wrote: On 08/14/2013 11:33 PM, Chris Hegarty wrote: * @throws ArrayIndexOutOfBoundsException * if the {@code off} is negative, or the {@code len} is * negative, or the {@code off+len} is greater than the * length of the

hg: jdk8/tl/jdk: 7154662: {CRC32, Adler32}.update(byte[] b, int off, int len): undocumented ArrayIndexOutOfBoundsException

2013-08-15 Thread xueming . shen
Changeset: 6c307b4d0dd8 Author:sherman Date: 2013-08-15 10:41 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/6c307b4d0dd8 7154662: {CRC32, Adler32}.update(byte[] b, int off, int len): undocumented ArrayIndexOutOfBoundsException Summary: to add the throws clause Reviewed-by:

RFR: JDK-7154662: {CRC32, Adler32}.update(byte[] b, int off, int len): undocumented ArrayIndexOutOfBoundsException

2013-08-14 Thread Xueming Shen
Hi, Please help review the change to document the AIOBE exception for CRC32/Adler32.update() methods (may need a followup CCC). This has been the behavior probably from day one, but the exception is just not explicitly listed. Ideally it probably should be in Checksum.update(), but that

Re: RFR: JDK-7154662: {CRC32, Adler32}.update(byte[] b, int off, int len): undocumented ArrayIndexOutOfBoundsException

2013-08-14 Thread Xueming Shen
) : If off is negative, len is negative, or len is greater than b.length - off Either is ok with me. Is it worth adding a test? or will JCK be updated to catch this. -Chris. On 14/08/2013 20:10, Xueming Shen wrote: Hi, Please help review the change to document the AIOBE exception for CRC32

Re: Please review java.util.formatter bug fix

2013-08-13 Thread Xueming Shen
Looks fine. Since you have already turned the verbose off and reduced the locales, it might be worth still letting it run N=12. The original idea with 12 is that it increases the chance of running into daylight saving on and off cases. -Sherman On 08/13/2013 12:06 PM, roger riggs wrote: Hi,

hg: jdk8/tl/jdk: 8020054: (tz) Support tzdata2013d

2013-08-09 Thread xueming . shen
Changeset: a7c341f30747 Author:sherman Date: 2013-08-09 12:40 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/a7c341f30747 8020054: (tz) Support tzdata2013d Summary: update the jdk8 tz data to version 2013d Reviewed-by: coffeys, peytoia ! make/sun/javazic/tzdata/VERSION !

hg: jdk8/tl/jdk: 8015666: test/tools/pack200/TimeStamp.java failing

2013-08-08 Thread xueming . shen
Changeset: a388263a7287 Author:sherman Date: 2013-08-08 12:03 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/a388263a7287 8015666: test/tools/pack200/TimeStamp.java failing Summary: to keep the default behavior of ZOS unchanged, if ze extra time not explicitly set

Re: RFR: 6614237: missing codepage Cp290 at java runtime

2013-08-08 Thread Xueming Shen
. cp290 EBCDIC-JP-kana csIBM290 The alias table should include EBCDIC-JP-kana and csIBM290 as well? Otherwise, the changes look good to me. Thanks, Masayoshi On 8/3/2013 5:53 AM, Xueming Shen wrote: Masayoshi, Would you please help review this change? The changeset also adds the requested

Re: Code review request: 8021788: JarInputStream doesn't provide certificates for some file under META-INF

2013-08-08 Thread Xueming Shen
Looks fine. -Sherman On 08/04/2013 06:28 PM, Weijun Wang wrote: Please take a look at http://cr.openjdk.java.net/~weijun/8021788/webrev.00/ The problem is that the method treats no META-INF entry as normal. If we can be sure that MANIFEST.MF and signature-related files are always at the

RFR JDK-8020054: (tz) Support tzdata2013d

2013-08-08 Thread Xueming Shen
Hi, Please help review the proposed change to update the tz data in JDK8 from 2013c to 2013d. http://cr.openjdk.java.net/~sherman/8020054/webrev http://cr.openjdk.java.net/~sherman/8020054/closed Tests list below have been run and passed (except

Re: RFR: 8015666: test/tools/pack200/TimeStamp.java failing

2013-08-02 Thread Xueming Shen
Kumar, Alan, Here is the latest webrev of the changes based on the latest CCC. http://cr.openjdk.java.net/~sherman/8015666/webrev -Sherman On 07/02/2013 11:29 AM, Xueming Shen wrote: On 06/28/2013 07:47 AM, Kumar Srinivasan wrote: Some nits while reading the changes: 1. ZipEntry.java

RFR: 6614237: missing codepage Cp290 at java runtime

2013-08-02 Thread Xueming Shen
Masayoshi, Would you please help review this change? The changeset also adds the requested Cp300. webrev: http://cr.openjdk.java.net/~sherman/6614237/webrev/ original mapptables: http://cr.openjdk.java.net/~sherman/6614237/290/ http://cr.openjdk.java.net/~sherman/6614237/300/ regression

hg: jdk8/tl/jdk: 8021767: test/java/time/tck/java/time/format/TCKFormatStyle.java failing

2013-07-30 Thread xueming . shen
Changeset: 8bc1bbd5b659 Author:sherman Date: 2013-07-30 14:43 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/8bc1bbd5b659 8021767: test/java/time/tck/java/time/format/TCKFormatStyle.java failing Summary: Correct to use fixed locale, not locale of test environment

hg: jdk8/tl/jdk: 8016025: JSR 310 DateTime API Updates IV; ...

2013-07-18 Thread xueming . shen
Changeset: b39797bb86c0 Author:sherman Date: 2013-07-18 11:02 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/b39797bb86c0 8016025: JSR 310 DateTime API Updates IV 8020418: Cleanup of -Xlint warnings in java.time 8016623: test/java/time/format/TestDateTimeTextProvider.java

RFR: 8016025 JSR 310: DateTime API Updates IV

2013-07-12 Thread Xueming Shen
Hi Please help review the update for the JSR310 DateTime API updates IV http://cr.openjdk.java.net/~sherman/8016025/webrev/ Here is the summary of the updates included in this round of API update: - Remove type parameter from ChronoLocalDate as unnecessary to simplify API and usage -

Re: RFR: 8015666: test/tools/pack200/TimeStamp.java failing

2013-07-02 Thread Xueming Shen
On 06/28/2013 07:47 AM, Kumar Srinivasan wrote: Some nits while reading the changes: 1. ZipEntry.java a. typo: + * Sets the laste access time of the entry. b. extra space +case EXTID_ZIP64 : 2. ZipOutputStream.java I think it would be nice to have the flags 0x1,

Re: RFR: 8015666: test/tools/pack200/TimeStamp.java failing

2013-06-27 Thread Xueming Shen
On 06/27/2013 10:04 AM, Kumar Srinivasan wrote: Hi Sherman, I started looking at this, my initial comment, the Unpacker.unpack does not close its output and we allow multiple pack files to be concatenated, I am assuming out.finish() will allow further jar files to be appended ? or would this

Re: JDK 8 request for review: remove final doclint warnings from java.util.zip

2013-06-26 Thread Xueming Shen
looks fine. thanks for taking care of this. -Sherman On 6/26/13 6:23 PM, Joe Darcy wrote: Hello, The java.util.zip package has a few remaining doctlint warnings; the patch below removes them. Thanks, -Joe diff -r 1fda8fa7ae97 src/share/classes/java/util/zip/Deflater.java ---

Re: Review request for JDK-8016760: failure of regression test langtools/tools/javac/T6725036.java

2013-06-25 Thread Xueming Shen
This is fine to be a workaround for the test case for now. It probably will need to be undo-ed after the propose change for #8015666 get integrated. http://cr.openjdk.java.net/~sherman/8015666/webrev/ The proposal for #8015666 is to keep the existing behavior of ZipEntry.getTime() to return a

Re: Review request for JDK-8016760: failure of regression test langtools/tools/javac/T6725036.java

2013-06-25 Thread Xueming Shen
the ms-dos formatted date/time has 2-second granularity, but the new one has second granularity, so the test case may still fail in some odd timing. -Sherman On 06/25/2013 11:33 AM, Eric McCorkle wrote: Is this a capital-R review? On 06/25/13 13:50, Xueming Shen wrote: This is fine

Re: Review request for JDK-8016760: failure of regression test langtools/tools/javac/T6725036.java

2013-06-25 Thread Xueming Shen
stop the error from happening? If so, then I'll withdraw this RFR. On 06/25/13 13:50, Xueming Shen wrote: This is fine to be a workaround for the test case for now. It probably will need to be undo-ed after the propose change for #8015666 get integrated. http://cr.openjdk.java.net/~sherman

Re: RFR: 8015666: test/tools/pack200/TimeStamp.java failing

2013-06-21 Thread Xueming Shen
On 6/21/13 6:33 AM, Alan Bateman wrote: On 20/06/2013 19:48, Xueming Shen wrote: : I'm proposing the following approach to add the functionality of supporting the utc-date/time-with-1-second granularity and keep the old behavior of the get/setTime() of the ZipEntry. (1) keep the time

hg: jdk8/tl/jdk: 8015728: (zipfs) demo/zipfs/basic.sh failing

2013-06-07 Thread xueming . shen
Changeset: aed2ad905da6 Author:sherman Date: 2013-06-07 13:49 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/aed2ad905da6 8015728: (zipfs) demo/zipfs/basic.sh failing Summary: to return the correct loc entry size from wirteLOC(); Reviewed-by: alanb !

RFR JDK-8015728: (zipfs) demo/zipfs/basic.sh failing

2013-06-06 Thread Xueming Shen
Hi, Please help review the proposed change to fix the regression triggered by the change for #6303183, in which the change failed to update the written loc entry size correctly when the NTFS timestamp is used on Windows platform (return size at line#2200 is not updated accordingly)

Re: RFR: JDK-8015813: add test/tools/pack200/TimeStamp.java to ProblemsList

2013-06-03 Thread Xueming Shen
looks fine. On 06/03/2013 08:31 AM, Kumar Srinivasan wrote: Hi, Please review adding a failing test to ProblemList, until Sherman and I determine the correct fix for the underlying problem in zip/pack200. Thanks Kumar diff --git a/test/ProblemList.txt b/test/ProblemList.txt ---

Re: RFR JDK-8015271: Conversion table for EUC-KR is incorrect

2013-05-30 Thread Xueming Shen
On 05/30/2013 05:31 AM, Alan Bateman wrote: On 29/05/2013 18:07, Xueming Shen wrote: Hi, Here is proposal to add one mapping entry into the mapping table for EUC-KR to add the newly added KS X 1001:2002 code point 2-71 (EUC_KR:0xA2E8 Unicode: U+0x327E) Info regarding this code point http

hg: jdk8/tl/jdk: 8015271: Conversion table for EUC-KR is incorrect

2013-05-30 Thread xueming . shen
Changeset: b588955b7e5b Author:sherman Date: 2013-05-30 14:47 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/b588955b7e5b 8015271: Conversion table for EUC-KR is incorrect Summary: to add the requested postal code mark character u+327e Reviewed-by: alanb !

RFR JDK-8015271: Conversion table for EUC-KR is incorrect

2013-05-29 Thread Xueming Shen
Hi, Here is proposal to add one mapping entry into the mapping table for EUC-KR to add the newly added KS X 1001:2002 code point 2-71 (EUC_KR:0xA2E8 Unicode: U+0x327E) Info regarding this code point http://www.unicode.org/L2/L2004/04267-n2815.pdf Here is the mapping change webrev

Re: RFR 6303183: Support NTFS and Unix-style timestamps for entries in Zip files

2013-05-29 Thread Xueming Shen
Hi Martin, Somehow most of your comments are blank lines in my mailbox, just wonder if you can send them again. Based on the hints, I updated those javadoc comments in ZipUtils to **. The code has been updated slightly (mainly to not output the extra time stamp if it already exists in the

hg: jdk8/tl/jdk: 4759491: method ZipEntry.setTime(long) works incorrectly; ...

2013-05-29 Thread xueming . shen
Changeset: 90df6756406f Author:sherman Date: 2013-05-29 19:50 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/90df6756406f 4759491: method ZipEntry.setTime(long) works incorrectly 6303183: Support NTFS and Unix-style timestamps for entries in Zip files 7012856: (zipfs) Newly

hg: jdk8/tl/jdk: 8001750: CharsetDecoder.replacement should not be changeable except via replaceWith method

2013-05-28 Thread xueming . shen
Changeset: 7fa2d1dcb8f6 Author:sherman Date: 2013-05-28 10:42 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/7fa2d1dcb8f6 8001750: CharsetDecoder.replacement should not be changeable except via replaceWith method Summary: to make defensive copy for set/get replacement byte

RFR JDK-8001750: CharsetDecoder.replacement should not be changeable except via replaceWith method

2013-05-24 Thread Xueming Shen
Hi, Please help review the change for JDK-8001750: CharsetDecoder.replacement should not be changeable except via replaceWith method http://cr.openjdk.java.net/~sherman/8001750/webrev/ The bug description is actually not accurate, the replacement field of CharsetDecoder class is String, not

hg: jdk8/tl/jdk: 8004789: (zipfs) zip provider doesn't work correctly with file systems providers rather than the default

2013-05-20 Thread xueming . shen
Changeset: 6a9148865139 Author:sherman Date: 2013-05-20 11:56 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/6a9148865139 8004789: (zipfs) zip provider doesn't work correctly with file systems providers rather than the default Summary: to use Files.createTempFile(...) to

hg: jdk8/tl/jdk: 8013730: JSR 310 DateTime API Updates III

2013-05-15 Thread xueming . shen
Changeset: ef04044f77d2 Author:sherman Date: 2013-05-15 07:48 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/ef04044f77d2 8013730: JSR 310 DateTime API Updates III Summary: Integration of JSR310 Date/Time API update III Reviewed-by: naoto Contributed-by:

Re: RFR [8014657] CheckedInputStream.skip allocates temporary buffer on every call

2013-05-15 Thread Xueming Shen
public long skip(long n) throws IOException { -byte[] buf = new byte[512]; +if (tmpbuf == null) { +tmpbuf = new byte[512]; +} long total = 0; while (total n) { long len = n - total; -len = read(buf, 0, len

RFR 8014217: Base64.getXDecoder().wrap(...).read() doesn't throw exception for an incorrect number of padding chars in the final unit

2013-05-14 Thread Xueming Shen
Hi, Please help review the change for handling a malformed base64 stream corner case. The latest spec has been updated/clarified as If there is padding character present in the final unit, the correct number of padding character(s) must be present, otherwise IllegalArgumentException is thrown

Re: RFR 8014217: Base64.getXDecoder().wrap(...).read() doesn't throw exception for an incorrect number of padding chars in the final unit

2013-05-14 Thread Xueming Shen
It has been discussed before that the IOE is preferred in case of the wrapped decoding stream. The spec of wrap() does mention that IOE when reading bytes that can not be decoded. So personally I feel it has been covered, but if preferred, it can go further as If there is padding character

Re: RFR 8012326: Deadlock occurs when Charset.availableCharsets() is called by several threads at the same time

2013-05-14 Thread Xueming Shen
: On 5/13/13 7:02 PM, Xueming Shen wrote: Hi Mandy, Here is the updated simple version as we chatted. I will leave the ExtendedCharsets rewrite for next round. http://cr.openjdk.java.net/~sherman/8012326/webrev/ http://cr.openjdk.java.net/%7Esherman/8012326/webrev/ thanks for the update

Re: RFR 8014217: Base64.getXDecoder().wrap(...).read() doesn't throw exception for an incorrect number of padding chars in the final unit

2013-05-14 Thread Xueming Shen
Thanks Chris, I updated the wording as Alan suggest to just make sure there is no doubt on the behavior :-) http://cr.openjdk.java.net/~sherman/8014217/webrev/ -Sherman On 05/14/2013 12:46 PM, Chris Hegarty wrote: Thanks for the explanation Sherman. No further changes required. What you have

hg: jdk8/tl/jdk: 8012326: Deadlock occurs when Charset.availableCharsets() is called by several threads at the same time

2013-05-14 Thread xueming . shen
Changeset: 08ef70f60e0d Author:sherman Date: 2013-05-14 14:09 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/08ef70f60e0d 8012326: Deadlock occurs when Charset.availableCharsets() is called by several threads at the same time Summary: removed the race condition risk from

hg: jdk8/tl/jdk: 8014217: Base64.getXDecoder().wrap(...).read() doesn't throw exception for an incorrect number of padding chars in the final unit

2013-05-14 Thread xueming . shen
Changeset: c70fff3db913 Author:sherman Date: 2013-05-14 14:20 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/c70fff3db913 8014217: Base64.getXDecoder().wrap(...).read() doesn't throw exception for an incorrect number of padding chars in the final unit Summary: to throw IOE

Re: RFR 8012326: Deadlock occurs when Charset.availableCharsets() is called by several threads at the same time

2013-05-13 Thread Xueming Shen
providers to extend. Just looking at the current implementation, it's not needed. More comments inlined below. On 5/10/2013 10:53 AM, Xueming Shen wrote: Thanks for the review. I have updated the Charset.java to use the init-on-depand holder idiom. L440: return

Re: RFR 8012326: Deadlock occurs when Charset.availableCharsets() is called by several threads at the same time

2013-05-13 Thread Xueming Shen
Hi Mandy, Here is the updated simple version as we chatted. I will leave the ExtendedCharsets rewrite for next round. http://cr.openjdk.java.net/~sherman/8012326/webrev/ Thanks! -Sherman On 5/13/13 3:01 PM, Xueming Shen wrote: Thanks Mandy for reviewing. Webrev has been updated to (1

hg: jdk8/tl/jdk: 8013386: (tz) Support tzdata2013c

2013-05-13 Thread xueming . shen
Changeset: ae35fdbab949 Author:sherman Date: 2013-05-13 20:35 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/ae35fdbab949 8013386: (tz) Support tzdata2013c Summary: updated tz data to version 2013c Reviewed-by: peytoia, okutsu ! make/sun/javazic/tzdata/VERSION !

Re: RFR [7021870] GzipInputStream closes underlying stream during reading

2013-05-13 Thread Xueming Shen
SequenceInputStream, which can close the stream during the read() call. And this is what my fix was about - to prevent closing of the underlying stream. Sincerely yours, Ivan On 11.05.2013 2:48, Xueming Shen wrote: The proposed fix does solve the issue. However it appears it does not fix the root cause

Re: i18n dev RFR JDK-8013386: (tz) Support tzdata2013c

2013-05-10 Thread Xueming Shen
On 5/10/2013 8:24 AM, Xueming Shen wrote: Hi Sean, Thanks for the review. It appears I missed jdk/test/sun/util/calendar/zi/tzdata, webrev has been updated to include the test data update. http://cr.openjdk.java.net/~sherman/8013386/webrev I will update TCKZoneRulesProvider.java separately

Re: RFR 8012326: Deadlock occurs when Charset.availableCharsets() is called by several threads at the same time

2013-05-10 Thread Xueming Shen
the DEC02XX and ENC02XX variable. Mandy On 5/4/2013 12:05 PM, Xueming Shen wrote: Thanks Ulf! There is another version with a new ExtendedCharsets.java at http://cr.openjdk.java.net/~sherman/8012326/webrev.newECP/ I merged the stuff in AbstractCharsetProvider into ExtendedCharsets.java

Re: RFR [7021870] GzipInputStream closes underlying stream during reading

2013-05-10 Thread Xueming Shen
The proposed fix does solve the issue. However it appears it does not fix the root cause that triggers this problem. The root cause, or the direct trigger of this bug is the fact that ZipInputStream .availble() is really NOT reliable to tell us whether or not there is any byte available to be

Re: RFR JDK-8013386: (tz) Support tzdata2013c

2013-05-09 Thread Xueming Shen
. Above points are not necessarily related to 2013c update and should be cleaned up separately perhaps. regards, Sean. On 08/05/2013 23:20, Xueming Shen wrote: Hi, Please help review the proposed change to update the tz data in JDK8 from 2012i to 2013c. Other than the tzdb data file update under

Re: RFR 8012326: Deadlock occurs when Charset.availableCharsets() is called by several threads at the same time

2013-05-08 Thread Xueming Shen
On 05/08/2013 11:58 AM, Alan Bateman wrote: On 04/05/2013 20:05, Xueming Shen wrote: : Another sync need is for the init(), in which it may update the aliasMap, classMap and aliasNameMap via charset() and deleteCharset() if those special properties are defined. There are two sources

Re: RFR 8012326: Deadlock occurs when Charset.availableCharsets() is called by several threads at the same time

2013-05-04 Thread Xueming Shen
from: https://bugs.openjdk.java.net/show_bug.cgi?id=100092 https://bugs.openjdk.java.net/show_bug.cgi?id=100095 -Ulf Am 03.05.2013 06:29, schrieb Xueming Shen: Hi, Please help review the proposed fix for 8012326. The direct trigger is the fix we put in #6898310 [1], in which we added

Re: RFR 8012326: Deadlock occurs when Charset.availableCharsets() is called by several threads at the same time

2013-05-03 Thread Xueming Shen
On 5/3/13 7:18 AM, Alan Bateman wrote: On 03/05/2013 05:29, Xueming Shen wrote: : An alternative is to really implement the singleton pattern in ECP, however given the existing mechanism we have in Charset.java and the hacky init() implementation we need in ECP, I go with the current

Re: Review Request for JDK-8003992: File and other classes in java.io do not handle embedded nulls properly

2013-05-03 Thread Xueming Shen
On 5/3/13 6:48 AM, Alan Bateman wrote: On 03/05/2013 00:08, Dan Xu wrote: Hi All, Thanks for all your comments. Based on the previous feedback, I have moved to the other approach, i.e., to fail file operations if the invalid NUL characher is found in a file path. As you know, due to the

RFR 8012326: Deadlock occurs when Charset.availableCharsets() is called by several threads at the same time

2013-05-02 Thread Xueming Shen
Hi, Please help review the proposed fix for 8012326. The direct trigger is the fix we put in #6898310 [1], in which we added the synchronization to prevent a possible race condition as described in $4685305. However it appears the added synchronization triggers another race condition as

RFR JDK-8013254: Constructor \w need update to add the support of \p{Join_Control}

2013-04-30 Thread Xueming Shen
Hi, It appears we dropped the ball on u+200c and u+200d when we updated the simple word boundaries back to jdk7 [1]. You can find most of the related discussion here [2]. These 2 code points are listed as one of the issues we were trying to fix but obviously the final doc and implementation

Re: RFR 8013252: Regex Matcher .start and .end should be accessible by group name

2013-04-30 Thread Xueming Shen
, Xueming Shen wrote: Hi, The regex named capturing group support was added into jdk7 [1]. Matcher.group(gname) is the only direct access method we added back then to access the matched result. The proposed change here is to add a pair of accessing/convenient method Matcher.start/end(gname) to access

Re: RFR JDK-8012645: Stream methods on BitSet, Random, ThreadLocalRandom, ZipFile

2013-04-30 Thread Xueming Shen
On 04/30/2013 01:32 PM, Henry Jen wrote: That's what it was, but as Alan pointed out, it is potential misleading to people who knows detail about zip file. I think the ordering is an implementation detail, and based on zip file spec, I would assume it's the order as in central directory. I

Re: RFR 8013252: Regex Matcher .start and .end should be accessible by group name

2013-04-30 Thread Xueming Shen
On 04/30/2013 01:58 PM, Mandy Chung wrote: On 4/30/13 1:22 PM, Xueming Shen wrote: updated webrev: http://cr.openjdk.java.net/~sherman/8013252/webrev/ Looks good. (again, the RegExTest.java is mixed with the change for #8013254) Are you going to separate the change in the right changeset

Re: RFR JDK-8013254: Constructor \w need update to add the support of \p{Join_Control}

2013-04-30 Thread Xueming Shen
My apology, the webrev is at http://cr.openjdk.java.net/~sherman/8013254/webrev/ -Sherman On 04/30/2013 10:01 AM, Xueming Shen wrote: Hi, It appears we dropped the ball on u+200c and u+200d when we updated the simple word boundaries back to jdk7 [1]. You can find most of the related

RFR 8013252: Regex Matcher .start and .end should be accessible by group name

2013-04-29 Thread Xueming Shen
Hi, The regex named capturing group support was added into jdk7 [1]. Matcher.group(gname) is the only direct access method we added back then to access the matched result. The proposed change here is to add a pair of accessing/convenient method Matcher.start/end(gname) to access the start/end

<    3   4   5   6   7   8   9   10   11   12   >