Re: RFR: 8222532: (zipfs) Performance regression when writing ZipFileSystem entries in parallel

Alan, Claes, If my memory was correct, ExChannelClose was not being used even in previous implementation. Actually it should never have been used in any of the released JDK. I might have been missing some discussions ... so I might be wrong here. But the original reason of having a

Re: ZipFileSystem performance regression

understand the motivation for the change, I don’t get why you would want to use ZipFs for what in essence is a RAM disk, *unless* you want it compressed in memory? Oh well. Do we need a new option for this? /Lennart Börjeson Electrogramma ab iPhono meo missum est 16 apr. 2019 kl. 21:44 skrev Xu

Re: ZipFileSystem performance regression

One of the motivations back then is to speed up the performance of accessing those entries, means you don't have to deflate/inflate those new/updated entries during the lifetime of that zipfilesystem. Those updated entries only get compressed when go to storage. So the regression is more

Re: JDK-8215626 : Correct [^..&&..] intersection negation behaviour JDK8 vs JDK11 ??

Hi Andrew, See [1]/[2] for the background of the fix. I would say jdk11 behavior is correct and expected :-) anyway, it's a  behavior change, so probably will not be easily to go back into jdk8. Regards, Sherman [1]

Re: RFR (S): 8215472: Cleanups in implementation classes of jdk.zipfs and tests

On 12/18/18 5:44 AM, Langer, Christoph wrote: In ZipFileSystem you remove the unused method releaseDeflater - to me this indicates the resource pooling is actually broken? I.e., shouldn't EntryOutputStreamDef return its Deflater to the cache when it's closed, similar to how the anonymous

Re: RFR 8212129: Remove finalize methods from java.util.zip.ZipFIle/Inflator/Deflator

+1 A potential "enhancement" for future consideration is whether or not worth to combine DeflaterZFStreamRef/InflaterZFStream back to one class, maybe with a "instanceof" before calling Deflater/Inflater.end(addr). -Sherman On 10/24/18, 10:19 AM, Lance Andersen wrote: Hi all, This change

Re: Fwd: RFR: JDK-8212694 - Using Raw String Literals with align() and Integer.MIN_VALUE causes out of memory error

looks good. -sherman *From: *Jim Laskey > *Subject: **RFR: JDK-8212694 - Using Raw String Literals with align() and Integer.MIN_VALUE causes out of memory error* *Date: *October 19, 2018 at 2:50:19 PM ADT *To: *core-libs-dev

Re: RFR (XS) 8211396 : Broken link in javadoc for private java.util.regex.Pattern#normalize()

+1 On 10/10/18, 10:22 AM, Ivan Gerasimov wrote: Hello! The javadoc refers to the enum values as java.text.Normalizer.Form.NFC, while it should be java.text.Normalizer.Form#NFC. Also an unpaired parenthesis was removed. Would you please help review the trivial fix? BUGURL:

Re: RFR (Enhancement): 6194856: Zip Files lose ALL ownership and permissions of the files

Hi Langer, Here are some of my comments. ZipEntry: Optional> getPosixPermissions() (1) Is "Optional" really necessary here. it's a little inconsistent compared to the rest of methods in the class. a "null" return might be just fine? (2) Needs a to separate the first line of

RFR JDK-8211880,Broken links in java.util.jar

Hi, Please help review the doc-link-only update for JDK-8211880 issue: https://bugs.openjdk.java.net/browse/JDK-8211880 webrev: http://cr.openjdk.java.net/~sherman/8211880/webrev/ Thanks, Sherman

Re: RFR JDK-8211728, JarFile::versionedStream() does not filter META-INF resources in versioned stream

On 10/6/18, 1:54 AM, Alan Bateman wrote: On 06/10/2018 09:29, Xueming Shen wrote: The created mr jar file is being tested in #139 with versionedStream(). Without the fix there will be a npe. Right but it's not testing that the versioned stream doesn't include META-INF/Foo.class. We need some

Re: RFR JDK-8211728, JarFile::versionedStream() does not filter META-INF resources in versioned stream

The created mr jar file is being tested in #139 with versionedStream(). Without the fix there will be a npe. On Oct 6, 2018, at 12:17 AM, Alan Bateman wrote: > On 05/10/2018 17:48, Xueming Shen wrote: > Hi > > Please help review the change for JDK-8211728 >

RFR JDK-8211728,JarFile::versionedStream() does not filter META-INF resources in versioned stream

Hi Please help review the change for JDK-8211728 issue: https://bugs.openjdk.java.net/browse/JDK-8211728 webrev: http://cr.openjdk.java.net/~sherman/8211728/webrev The "intention" of JEP 238 is that the resources under META-INF can't/shouldn't be versioned. But it appears this is not

Re: [PATCH] JDK-8211765 - JarFile constructor throws undocumented exception

Hi Jaikiran, "wrap to throw a IOE" appears good. I will sponsor this patch. Thanks! -Sherman On 10/5/18, 8:31 AM, Jaikiran Pai wrote: Thank you Chris. I've attached a patch here which catches the InvalidPathException within the implementation of ZipFile and wraps and throws a IOException.

Re: Review Request: 6202130: java.util.jar.Attributes.writeMain() can't handle multi-byte chars

+1 only nit is it might be preferred to use "utf8" directly in the test instead of current "utf" -sherman On 10/03/2018 11:55 AM, Philipp Kunz wrote: In short, bug 6202130 is about removal of comments in Attributes like XXX Need to handle UTF8 values and break up lines longer than 72 The

Re: RFR JDK-8211385: (zipfs) ZipDirectoryStream yields a stream of absolute paths when directory is relative

On 10/3/18, 6:43 AM, Alan Bateman wrote: On 02/10/2018 21:04, Xueming Shen wrote: Hi, Please help review the change for JDK-8211585. issue: https://bugs.openjdk.java.net/browse/JDK-8211385 webrev: http://cr.openjdk.java.net/~sherman/8211385/webrev This looks okay although it might

Re: RFR: 8211382 ISO2022JP and GB18030 NIO converter issues

+1 -Sherman btw, since gb18030.decodeArrayLoop() is right I would assume it's just a "typo" in decodeBufferLoop() On 10/2/18, 2:21 AM, Ichiroh Takiguchi wrote: Hello, IBM would like to contribute NIO converter patch to OpenJDK project. Bug:

RFR JDK-8211385: (zipfs) ZipDirectoryStream yields a stream of absolute paths when directory is relative

Hi, Please help review the change for JDK-8211585. issue: https://bugs.openjdk.java.net/browse/JDK-8211385 webrev: http://cr.openjdk.java.net/~sherman/8211385/webrev Thanks, Sherman

Re: RFR (Enhancement): 6194856: Zip Files lose ALL ownership and permissions of the files

Hi Langer, Thanks for working on this issue. I will take a look into the implementation details next week. Here are two comments regarding the "direction/approach". (1) There is a "masked" security concern regarding adding the "ownership/permission" into the jar file. "Security concern:

Re: RFR: 8211163: UNIX version of Java_java_io_Console_echo does not return a clean boolean

+1 On 9/26/18, 9:14 AM, Alan Bateman wrote: On 26/09/2018 16:58, Alan Bateman wrote: On 26/09/2018 16:51, Andrew Haley wrote: Java_java_io_Console_echo looks like this: JNIEXPORT jboolean JNICALL Java_java_io_Console_echo(JNIEnv *env, jclass cls, jboolean on) { DWORD fdwMode;

Re: RFR JDK-8210899: (zipfs) ZipFileSystem.EntryOutputStreamCRC32 mistakenly set the crc32 value into size field

Thanks, Brian On Sep 18, 2018, at 5:14 PM, Xueming Shen <mailto:xueming.s...@oracle.com>> wrote: Please help codereview the change for JDK-8210899. issue:https://bugs.openjdk.java.net/browse/JDK-8210899 webrev:http://cr.openjdk.java.net/~sherman/8210899/webrev <http://cr.open

RFR JDK-8210899: (zipfs) ZipFileSystem.EntryOutputStreamCRC32 mistakenly set the crc32 value into size field

Hi, Please help codereview the change for JDK-8210899. issue: https://bugs.openjdk.java.net/browse/JDK-8210899 webrev: http://cr.openjdk.java.net/~sherman/8210899/webrev Thanks, Sherman

Re: RFR 12 : 8072130 : java/lang/instrument/BootClassPath/BootClassPathTest.sh fails on Mac OSX

+1 On 9/18/18, 9:42 AM, Brent Christian wrote: Any thoughts on this change? -B On 9/11/18 3:41 PM, Brent Christian wrote: Hi, Please review this change to how the platform encoding is determined on Mac when loading agents. Webrev:

Re: RFR JDK-8034802: (zipfs) newFileSystem throws UOE when the zip file is located in a custom file system

> shouldn't it rather be "illegal position " as it must not necessarily be a negative position? Assuming that the updated test case runs correctly, you can consider the changes reviewed from my end. I'll post a change for adding executable bit support soon, based on your changes

RFR JDK-8034802: (zipfs) newFileSystem throws UOE when the zip file is located in a custom file system

Hi Please help review the change for JDK-8034802. issue: https://bugs.openjdk.java.net/browse/JDK-8034802 webrev: http://cr.openjdk.java.net/~sherman/8034802/webrev One of the reasons that the implementation works this way is that I didn't have a complete SeekableByteChannel support (mainly

Re: JDK-8124977: deadlock between engineers? (cmdline encoding challenges on Windows)

On 9/13/18, 3:36 PM, Xueming Shen wrote: Anthony, I don't see/recall there was any response to my last review comment [1]. The proposed change in webrev.05 [2], in which it forces the internal system property sun.jnu.encoding to be always "utf8", is incomplete and incorrect (see my

Re: JDK-8124977: deadlock between engineers? (cmdline encoding challenges on Windows)

Anthony, I don't see/recall there was any response to my last review comment [1]. The proposed change in webrev.05 [2], in which it forces the internal system property sun.jnu.encoding to be always "utf8", is incomplete and incorrect (see my comment [3] for why). file.encoding: how to

Re: RFR(S) 8190737: use unicode version of the canonicalize() function to handle long path on windows

;, or ANSI charset. Thanks, -Sherman On 9/12/18, 4:16 PM, Calvin Cheung wrote: Hi Sherman, Thanks for your review. Please refer to my reply below... On 9/10/18, 5:05 PM, Xueming Shen wrote: On 9/10/18, 11:42 AM, Calvin Cheung wrote: bug: https://bugs.openjdk.java.net/browse/JDK-8190737 we

Re: Re-iterate JDK-6194856: Zip Files lose ALL ownership and permissions of the files

It's hard to get the ZipEntry API right to perfectly handle these platform specific "file system attributes" especially given the potential performance and security concern. I would assume zipfs might be a better place to handle this if "really" desired, in which already has the base to

Re: RFR(S) 8190737: use unicode version of the canonicalize() function to handle long path on windows

On 9/10/18, 11:42 AM, Calvin Cheung wrote: bug: https://bugs.openjdk.java.net/browse/JDK-8190737 webrev: http://cr.openjdk.java.net/~ccheung/8190737/webrev.00/ Please review this change for handling long path specified in the -Xbootclasspath/a on windows. Highlight of changes: -

RFR JDK-8210345: The Japanese message of FileNotFoundException garbled

Hi Please help review the changeset for JDK-8210345. issue: https://bugs.openjdk.java.net/browse/JDK-8210345 webrev: http://cr.openjdk.java.net/~sherman/8210345/webrev This is a "regression" from the change we made for JDK-8200243 [1], in which the getLastErrorString() for windows platform

RFR JDK-8210394: (zipfs) jdk/nio/zipfs/ZFSTests.java rootdir.zip: The process cannot access the file because it is being used by another process

Hi Please help review the change for JDK-8210394. issue: https://bugs.openjdk.java.net/browse/JDK-8210394 webrev: http://cr.openjdk.java.net/~sherman/8210394/webrev It appears the "zipfile" for testing is opened with "new ZipFile(...).stream()..." without explicit close might be the direct

Re: RFR 8210285 : CharsetDecoder/Encoder's constructor does not reject NaN

+1 On 8/31/18, 5:17 PM, Ivan Gerasimov wrote: Hello! The javadoc for CharsetDecoder [1] states that an exception is thrown when a non-positive number is passed in as an argument. However we only reject negative or zero numbers, but not NaN. And likewise for CharsetEncoder. Would you

Re: RFR: JDK-8197398, (zipfs) Files.walkFileTree walk indefinitelly while processing JAR file with "/" as a directory inside.

On 8/29/18, 3:22 AM, Alan Bateman wrote: The approach looks okay, I think just wonder if the test could be expanded to cover entry with repeated leading slashes. One nit is that hasAbsolutePath (and also the existing readOnly) aren't final. One suggestion is for initCEN to return a CEN object

RFR: JDK-8197398, (zipfs) Files.walkFileTree walk indefinitelly while processing JAR file with "/" as a directory inside.

Hi, Please help review the proposed change for JDK-8197398. issue: https://bugs.openjdk.java.net/browse/JDK-8197398 webrev: http://cr.openjdk.java.net/~sherman/8197398/webrev A little background: The existing zipfs has an assumption that the "normal/healthy/secured" zip/jar file should not

Re: RFR: 8209937: Enhance java.io.Console - provide methods to query console width and height

To the core-libs ... On 8/24/18, 9:56 AM, Xueming Shen wrote: Hi Langer, I would suggest to add explicit wording to emphasize the "size" means the rows and columns in characters, not the pixels. It appears attractive to have a Console.Dimension to abstract the size and make it

Re: RFR(JDK12/NIO) 8209576: java.nio.file.Files.writeString writes garbled UTF-16 instead of UTF-8

+1 On 08/17/2018 02:39 PM, Joe Wang wrote: Hi, Please review a fix for a condition where a code conversion was skipped incorrectly resulting in a wrong byte array being written into the file. JBS: https://bugs.openjdk.java.net/browse/JDK-8209576 webrevs:

Re: Is returning a value != '0' or '1' as jboolean from a JNI function legal?

On 08/17/2018 10:25 AM, Aleksey Shipilev wrote: On 08/17/2018 05:12 PM, Volker Simonis wrote: The offending code in Console_md.c looks as follows: #define ECHO8 JNIEXPORT jboolean JNICALL Java_java_io_Console_echo(...) { jboolean old; ... old = (tio.c_lflag& ECHO);

Re: Add x-IBM-1129 charset

The change looks fine. Btw, do you guy have better implementation for these two charsets? These two were left untouched when I did the reimplementation back to 6/7. Did not have time to figure out its mapping table (especially the compatibility concern when dealing with the mapping hard-coded

Re: RFR : 8207395: jar has issues with UNC-path arguments for the jar -C parameter [windows]

future move of jar to a complete nio implementation !) Best regards, Matthias Message: 2 Date: Wed, 18 Jul 2018 11:37:34 -0700 From: Xueming Shen To: core-libs-dev@openjdk.java.net Subject: Re: RFR : 8207395: jar has issues with UNC-path arguments for the jar -C parameter [windows

Re: Console.readPassword() was failed on Linux s390x platform

Hi Ichiroh, Does the test case always fail? how about the previous version? The Console class was updated slightly in 11 though. does "old = (tio.c_lflag & ECHO) != 0" fix the issue as well? Please file a issue with appropriate P value. Not sure if it can make 11. Thanks, Sherman old =

Re: Adding new IBM extended charsets

To: Nasser Ebrahim , Xueming Shen , core-libs-dev@openjdk.java.net Date: 07/09/2018 01:25 AM Subject: Re: Adding new IBM extended charsets On 06/07/2018 14:56, Nasser Ebrahim wrote: > : > I understood you preferred

Re: RFR : 8207395: jar has issues with UNC-path arguments for the jar -C parameter [windows]

Ideally it would be preferred to move jar to a complete nio implementation and we then can leave the WindowsPath to take care of this stuff. That said that would be a relative big change. I do have a version on my disk but need clean up, test, performance measurement and review to get it.

Re: RFR JDK-8206389: JarEntry.setCreation/LastAccessTime without setLastModifiedTime causes Invalid CEN header

On 7/6/18, 5:43 PM, Martin Buchholz wrote: I would use different timestamps for the 4 possible times used in this test, if only to make it clearer which value comes from where. webrev has been updated accordingly. +// ze.setLastModifiedTime(now); +

RFR JDK-8206389: JarEntry.setCreation/LastAccessTime without setLastModifiedTime causes Invalid CEN header

Hi Please help review the changeset for JDK-8206389 issue: https://bugs.openjdk.java.net/browse/JDK-8206389 webrev: http://cr.openjdk.java.net/~sherman/8206389/webrev Cause: ZipOutputStream.writeCEN().writeCEN() incorrectly calculate the length of bytes needed for the "unix timestamps" when

Re: Adding new IBM extended charsets

On 7/4/18, 5:41 AM, Nasser Ebrahim wrote: Hello, Am starting this mail thread to discuss about adding new IBM extended charsets. The questions is whether we need to add the new extended charsets to jdk.charsets or to a new separate charset provider/module like jdk.ibmcharsets. This discussion

Re: RFR JDK-8200243: System error message is decoded as invalid encoding in Windows.

On 6/27/18, 12:39 AM, Alan Bateman wrote: On 27/06/2018 04:14, Xueming Shen wrote: Hi, Please help review the change for JDK-8200243 issue: https://bugs.openjdk.java.net/browse/JDK-8200243 webrev: http://cr.openjdk.java.net/~sherman/8200243/webrev This is a regression. The root cause is one

RFR JDK-8200243: System error message is decoded as invalid encoding in Windows.

Hi, Please help review the change for JDK-8200243 issue: https://bugs.openjdk.java.net/browse/JDK-8200243 webrev: http://cr.openjdk.java.net/~sherman/8200243/webrev This is a regression. The root cause is one of the change we put in jdk9 for JDK-805 [1], which is to remove those unused vm

Re: RFR(JDK11/NIO) 8205058 throw CharacterCodingException --> Re: RFR (JDK11/NIO) 8201276: (fs) Add methods to Files for reading/writing a string from/to a file

+1 On 6/26/18, 12:49 PM, Joe Wang wrote: On 6/26/18, 11:57 AM, Alan Bateman wrote: On 26/06/2018 18:41, Joe Wang wrote: : Yes, combined with Sherman's suggestion eliminated the need for the new parameter. Here's the updated webrev:

Re: RFR(JDK11/NIO) 8205058 throw CharacterCodingException --> Re: RFR (JDK11/NIO) 8201276: (fs) Add methods to Files for reading/writing a string from/to a file

Hi Joe, I would suggest always embed a malformed exception into the iae as showed below, then the extra flag is no longer needed. private static void throwMalformed(int off, int nb) { String msg = "malformed input off : " + off + ", length : " + nb; throw new

Re: RFR(S) JDK-8205528: Base64 Encode Algorithm using AVX512 Instructions

Hi Kamath, Instead of throwing an aiobe, should the generateImplEncode() be like void generateImplEncode(byte[] src, int sp, int sl, byte[] dst, int dp) { if (sp < sl) { implEncode(src, sp, sl, dst, dp, ...); } } Any benefit of separating it into its own method? Thanks,

Re: The store for byte strings

On 6/9/18, 3:27 AM, Florian Weimer wrote: Lately I've been thinking about string representation. The world turned out not to be UCS-2 or UTF-16, after all, and we often have to deal with strings generally encoded as ASCII or UTF-8, but we aren't always encoded this way (and there might not even

RFR JDK-8204229: Formatter and String.format ignore the width with the percent modifier (%5%)

Hi, Please help review the change for JDK-8204229. It appears to be a overlook in the implementation. We do have a method print(String, Locale) that adjust the "padding spaces" issue: https://bugs.openjdk.java.net/browse/JDK-8204229 webrev: http://cr.openjdk.java.net/~sherman/8204229/webrev

RFR https://bugs.openjdk.java.net/browse/JDK-8204494

Hi Please help review the change for JDK-8204494, a regression caused by the fix for JDK-8200530 [1]. It appears that fix fails to deal with the corner case that a pair of \r\n is at the boundary of the internal buf of Manifest.FastInputtStream, which is 8192 for the default. In that

Re: RFR JDK-8200530: '\r' is not supported as "newline" in java.util.jar.Manifest.

if ((c = tbuf[tpos-1]) == '\n' || c == '\r') { break; } == if (c == '\n' || c == '\r') { break; } <<<<<< +1 Cheers, — Jim On Jun 5, 2018, at 2:38 AM, Xueming Shen wrote:

RFR JDK-8200530: '\r' is not supported as "newline" in java.util.jar.Manifest.

Hi, Please help review the changeset for JDK-8200530. "newline" is specified as |CR LF | LF | CR|(/not followed by/|LF|) in Jar spec [1] from the very beginning but our implementation actually never supports "\r"/CR (not followed by LF) case. The proposed change here is to add CR as an

RFR: JDK-8203839: API clarification: versioned jar entry verification in multi-release jar file

Hi, Please help review the proposed api spec update for JDK-8203839 (and its CSR) issue: JDK-8203839: API clarification: versioned jar entry verification in multi-release jar file csr: https://bugs.openjdk.java.net/browse/JDK-8203840 webrev:

Re: RFR JDK-8200172,String.split non-positive term incorrect use

) ) Best Lance On May 22, 2018, at 7:07 PM, Xueming Shen <xueming.s...@oracle.com> wrote: Hi, Please help review a api doc clarification for String.split()/Pattern.split(). issue: https://bugs.openjdk.java.net/browse/JDK-8200172 webrev: http://cr.openjdk.java.net/~sherman/8200172/webr

RFR JDK-8200172,String.split non-positive term incorrect use

Hi, Please help review a api doc clarification for String.split()/Pattern.split(). issue: https://bugs.openjdk.java.net/browse/JDK-8200172 webrev: http://cr.openjdk.java.net/~sherman/8200172/webrev As suggested, it appears to be clear, straightforward and less confusion to simply

RFR JDK-8196987: Resolve disabled warnings for libzip

Hi Please help review the change for JDK-8196987. issue: https://bugs.openjdk.java.net/browse/JDK-8196987 webrev: http://cr.openjdk.java.net/~sherman/8196987/webrev It appears the change for JDK-6341887 in Deflater.c#169 "accidentally" fixed the offending "implicit-fallthrough" warning/error

Re: RFR: JDK-8200380 String::lines

On 5/18/18, 6:44 AM, Jim Laskey wrote: String::lines instance method that returns a Stream with elements composed of substrings from the original string delimited by any recognized new line character sequence. webrev:

Re: RFR: Rename EFS in java.util.zip internals to something meaningful

On 5/16/18, 6:28 PM, Martin Buchholz wrote: Hi Xueming, I'd like you to do a code review 8203328: Rename EFS in java.util.zip internals to something meaningful http://cr.openjdk.java.net/~martin/webrevs/jdk/zip-EFS/

Re: Discussion: Efficient ByteBuffer -> String that avoids additional copying

On 5/16/18, 10:03 AM, Claes Redestad wrote: Hi, I'd just like to point out there's been some recent effort on this, see: https://bugs.openjdk.java.net/browse/JDK-8021560 .. which was RFR'd here: http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-February/051401.html I'm not sure

Re: RFR JDK-8191533,jar --describe-module prints service provider class names in lower case

On 5/16/18, 12:19 AM, Alan Bateman wrote: On 16/05/2018 00:51, Xueming Shen wrote: Hi, Please help review the change for JDK-8191533 issue: https://bugs.openjdk.java.net/browse/JDK-8191533 webrev: http://cr.openjdk.java.net/~sherman/8191533/webrev/ The jmod change has been verify by applying

RFR JDK-8191533,jar --describe-module prints service provider class names in lower case

Hi, Please help review the change for JDK-8191533 issue: https://bugs.openjdk.java.net/browse/JDK-8191533 webrev: http://cr.openjdk.java.net/~sherman/8191533/webrev/ The jmod change has been verify by applying jmod on those modules at jdk home. thanks! sherman

Re: RFR: JDK-8200436 - String::isBlank

+1 On 5/14/18, 8:25 AM, Jim Laskey wrote: New string instance method that returns true if the string is empty or contains only white space, where white space is defined as any codepoint returns true when passed to Character::isWhitespace. webrev:

Re: [11] RFR: api/java_text/SimpleDateFormat/index.html#Format testcases started to fail with JDK11 b12

+1 On 05/10/2018 12:27 PM, naoto.s...@oracle.com wrote: Hello, Please review the fix to the following issue: https://bugs.openjdk.java.net/browse/JDK-8202764 The proposed fix is located at: http://cr.openjdk.java.net/~naoto/8202764/webrev.00/ This is a regression caused by 8181157, where

Re: [AIX] Add charset IBM-964 (default charset for zh_TW.IBM-eucTW) to stdcs

SimpleEUCEncoder is something leftover when I reimplemented the charsets to be map-based-built-during-build-time. I would recommend to leave them as Bhaktavatsal suggested for now. Ideally any new charsets added to the platform needs to be based on the new model

Re: RFR for 6443578 and 6202130: UTF-8 in Manifests

Philipp, I kinda recalled JDK-6202130 which I read long time ago and I believed it's not a bug but just wanted to leave it open and for double check, then it's off the priority list somehow... Reread the code below I think it handles utf8 and the 72-length-limit appropriately, though a

Re: RFR for 6443578 and 6202130: UTF-8 in Manifests

Hi Philipp, Thanks for your patience :-) I will get back to you with my feedback the next couple days. -Sherman On 5/2/18, 6:12 PM, Philipp Kunz wrote: Hi, Here is patch for 6443578 and 6202130 also in webrev form. http://files.paratix.ch/jdk/6372077and6443578/webrev.01/

Re: RFR: JDK-8202105: jshell tool: on exiting, terminal echo is disabled

+1 (nit: remove the "where:" and the corresponding indent before push please). On 4/26/18, 1:34 PM, Jan Lahoda wrote: Ok, here is an updated webrev: http://cr.openjdk.java.net/~jlahoda/8202105/webrev.02/ Jan

Re: [11] RFR: 8181157: CLDR Timezone name fallback implementation

+1 On 4/23/18, 6:25 PM, Naoto Sato wrote: Hi Sherman, thanks for the review. On 4/23/18 1:06 PM, Xueming Shen wrote: Naoto, Here some comments (1) CLDRTimeZoneNameProviderImpl.java: Ln#58: to use Stream.toArray(String[]::new) ? no sure which one is faster Ln#155-160

Re: RFR: JDK-8202105: jshell tool: on exiting, terminal echo is disabled

On 4/26/18, 3:23 AM, Jan Lahoda wrote: On 25.4.2018 22:59, Xueming Shen wrote: On 04/25/2018 01:39 PM, Jan Lahoda wrote: So, if I understand correctly, it would be: boolean flipEcho; and the readPassword would do something like: if (echo0() != false) { if (echo0()) { ... flipEcho

Re: RFR: JDK-8202105: jshell tool: on exiting, terminal echo is disabled

, 2018 at 9:34 AM, Xueming Shen <xueming.s...@oracle.com> wrote: On 4/25/18, 9:02 AM, Martin Buchholz wrote: It would be more correct I think for Console to track if there is a pending readPassword in progress and try to restore echo on exit only if so. But a little annoying to implement

Re: RFR: JDK-8202105: jshell tool: on exiting, terminal echo is disabled

"to add restoredEchoOnShutdown = true", and I'm fine with that. I'm just a little confused why jshell invokes System.console() if it handles all "raw" terminal stuff itself. On Wed, Apr 25, 2018 at 8:52 AM, Xueming Shen <xueming.s...@oracle.com <mailto:xueming.s...@ora

Re: RFR: JDK-8202105: jshell tool: on exiting, terminal echo is disabled

Hi Jan, I saw System.console() returns null inside jshell ... but it seems there are 2 vms. I would assume jshell itself sets the terminal to raw and then call System.console()? for example an alternative for this issue is to ask jshell's impl to call System.console() before going into raw

Re: [PATCH] regex matcher opt: remove redundant StringBuilder

nfo] origM avgt 10 11.669 ± 0.211 us/op [info] newM avgt 10 8.926 ± 0.095 us/op Isaac On Tue, Apr 24, 2018 at 12:53 PM, Xueming Shen<xueming.s...@oracle.com> wrote: Hi Isaac, I actually meant to say "we are not supposed to output the partial text into the output buffer in

Re: [PATCH] regex matcher opt: remove redundant StringBuilder

d(text, lastAppendPosition, first); // Append the match substitution +appendExpandedReplacement(replacement, sb); -sb.append(result); On Mon, Apr 23, 2018 at 5:05 PM Xueming Shen<xueming.s...@oracle.com> wrote: I would assume in case of an exception thrown from appendExp

Re: [PATCH] minor regex cleanup: use switch for enum

// Append the match substitution +appendExpandedReplacement(replacement, sb); -sb.append(result); On Mon, Apr 23, 2018 at 4:31 PM, Xueming Shen <xueming.s...@oracle.com <mailto:xueming.s...@oracle.com>> wrote: > this looks fine. > > -sherman

Re: [PATCH] minor regex cleanup: use switch for enum

this looks fine. -sherman On 4/23/18, 12:26 PM, Isaac Levy wrote: ping? Isaac On Wed, Apr 18, 2018 at 2:58 PM, Isaac Levy wrote: Hi, Minor improvement in readability (and probably perf) for Pattern. Switch is more consistent with the rest of the impl and the

Re: [11] RFR: 8181157: CLDR Timezone name fallback implementation

Naoto, Here some comments (1) CLDRTimeZoneNameProviderImpl.java: Ln#58: to use Stream.toArray(String[]::new) ? no sure which one is faster Ln#155-160: is it worth considering to check all possible empty slots in "names" here (from index_std_long to

Re: RFR: 6805750: Improve handling of Attributes.Name

Claes, It loogsk good. One minor nit is that the "hashCode" check in Name.equals() might not be really necessary, given in most/normal use scenario the "Name" is used as a key in Attributes.map, which is a LinkedHashMap. Arguably the hashcode should be check already during lookup. Agreed what

Re: [JDK-6341887] RFR: Patch V3: java.util.zip: Add ByteBuffer methods to Inflater/Deflater

v/ Thanks, Sherman On 4/18/18, 1:16 PM, David Lloyd wrote: +1 from me, thanks! On Wed, Apr 18, 2018 at 3:09 PM, Xueming Shen<xueming.s...@oracle.com> wrote: Alan, David, Any more update/comment/suggestion on this one? I have updated DeInflate.java with some new test cases to cover the new

Re: [JDK-6341887] RFR: Patch V3: java.util.zip: Add ByteBuffer methods to Inflater/Deflater

Alan, David, Any more update/comment/suggestion on this one? I have updated DeInflate.java with some new test cases to cover the newly added methods. Good to go? -Sherman On 04/10/2018 08:49 PM, Xueming Shen wrote: http://cr.openjdk.java.net/~sherman/6341887.David.Lloyd/webrev/ Thanks

Re: RFR: JDK-8194750,Console.readPassword does not save/restore tty settings

On 04/17/2018 12:13 AM, Alan Bateman wrote: On 17/04/2018 06:15, Xueming Shen wrote: Thanks! webrev has been updated accordingly as suggested. http://cr.openjdk.java.net/~sherman/8194750/webrev Just catching on this one. The changes looks good, I'm just wondering if there is any way

Re: RFR: JDK-8194750,Console.readPassword does not save/restore tty settings

Thanks! webrev has been updated accordingly as suggested. http://cr.openjdk.java.net/~sherman/8194750/webrev sherman On 4/16/18, 7:20 PM, Martin Buchholz wrote: Thanks, this looks good. But I have my usual nitpicky comments 376 // sets the console echo on/off 377 private static

Re: RFR: JDK-8194750,Console.readPassword does not save/restore tty settings

On 4/13/18, 8:04 PM, Martin Buchholz wrote: This is trickier than I expected, since you have to manage saving/restoring around each call to readPassword AND have an exit hook to restore in case the user never gets around to responding to the prompt. I see in the old code echo returns a

Re: RFR(S): 8201540: [AIX] Extend the set of supported charsets in java.base

It looks good to me. -Sherman On 4/16/18, 4:10 AM, Bhaktavatsal R Maram wrote: Hi All, I've regenerated webrev using "hg rename" to create template files. webrev looks much neat now.. Thanks Alan for suggestion. webrev - http://cr.openjdk.java.net/~gromero/8201540/v2/ Thanks, Bhaktavatsal

RFR: JDK-8194750, Console.readPassword does not save/restore tty settings

Hi, Please help review the change for JDK-8194750. issue: https://bugs.openjdk.java.net/browse/JDK-8194750 webrev: http://cr.openjdk.java.net/~sherman/8194750/webrev *fix has been manually verified. No new auto-regression test added. Thanks, Sherman

RFR: JDK-8201443: NoSuchMethodException JarFile.open when jar file is used in classpath

Hi, Please help review the change for JDK-8201443 issue: https://bugs.openjdk.java.net/browse/JDK-8201443 webrev: http://cr.openjdk.java.net/~sherman/8201443/webrev (1) To use SharedSecret/JavaLangAccess/Class.getDeclaredPublicMethods to avoid NoSuchMethodExcetpion throwing. (2) Given

Re: [JDK-6341887] RFR: Patch V3: java.util.zip: Add ByteBuffer methods to Inflater/Deflater

Hi David, The CSR has been approved https://bugs.openjdk.java.net/browse/JDK-8200527 API docs have been updated slightly based on the review suggestion. (1) added some words in the class spec for both Inflater and Deflater. |* * This class deflates sequences of bytes into ZLIB compressed

Re: RFR: 8184692: add Pattern.asMatchPredicate

Paul, I don't recall any particular reason why we did Predicate asPredicate() instead of Predicate asPredicate() back 1.8? mutability? sherman On 4/9/18, 8:41 AM, Roger Riggs wrote: Hi Vivek, As with Pattern.asPredicate the first sentence can be improved. Creates a predicate that

Re: [JDK-6341887] RFR: Patch V3: java.util.zip: Add ByteBuffer methods to Inflater/Deflater

else input.position(inputPos + inputConsumed; the if/else might be repetitive, but better than two layers of same DFE catch? -sherman On 3/30/18, 9:54 AM, David Lloyd wrote: Thanks! On Fri, Mar 30, 2018 at 11:52 AM, Xueming Shen<xueming.s...@oracle.com> wrote: On 3/30/18, 7:07

Re: [JDK-6341887] RFR: Patch V3: java.util.zip: Add ByteBuffer methods to Inflater/Deflater

On 3/30/18, 7:07 AM, Alan Bateman wrote: On 29/03/2018 13:18, David Lloyd wrote: : OK great. In that case, I think all feedback has been accounted for, and this should be ready to go AFAIK. I skimmed through the patch attached to your last mail. I also saw Sherman's mail offering to look at

Re: [JDK-6341887] RFR: Patch V3: java.util.zip: Add ByteBuffer methods to Inflater/Deflater

On 3/28/18, 6:14 AM, David Lloyd wrote: The current wording (which pre-dates your changes of course) reads more like API advice. I think it's a bit confusing too as it doesn't define what a "flush marker" is. I think we will need to re-word that sentence to make it clearer. OK. I am at a

Re: RFR: Some patches for sherman

looks good for me. On 3/26/18, 5:21 PM, Martin Buchholz wrote: 8200116: ConstructInflaterOutput, ConstructDeflaterInput still spamming test logs http://cr.openjdk.java.net/~martin/webrevs/jdk/ConstructInflaterOutput/

Re: [JDK-6341887] RFR: Patch V3: java.util.zip: Add ByteBuffer methods to Inflater/Deflater

.ll...@redhat.com> wrote: Sorry, that was an error on my part, caused by too much context switching. I've posted an update at https://github.com/dmlloyd/openjdk/commit/zlib-bytebuffer-v13 which is also attached. On Wed, Mar 14, 2018 at 8:53 PM, Xueming Shen<xueming.s...@oracle.com>

Re: [JDK-6341887] RFR: Patch V3: java.util.zip: Add ByteBuffer methods to Inflater/Deflater

edhat.com> wrote: On Fri, Mar 2, 2018 at 12:49 PM, Xueming Shen<xueming.s...@oracle.com> wrote: Hi David, (1) Deflater.deflate(Bytebuffer) the api doc regarding "no_flush" appears to be the copy/paste of the byte[] version without being updated to the corresp

Re: Raw String Literal Library Support

On 3/13/18, 5:12 PM, Jonathan Bluett-Duncan wrote: Paul, AFAICT, one sort of behaviour which String.split() allows which Pattern.splitAsStream() and the proposed String.splits() don't is allowing a negative limit, e.g. String.split(string, -1). Over at

Re: RFR JDK-8196748, tools/jar tests need to tolerate unrelated warnings

On 3/12/18, 11:57 PM, David Holmes wrote: test/jdk/tools/jar/modularJar/Basic.java + } else if (line.contains("Java HotSpot(TM) 64-Bit Server VM warning:")) { + continue; // ignore server vm warning see#8196748 That needs to be any VM warning as

  1   2   3   4   5   6   7   8   9   10   >