Re: RFR (trivial): 8211093: Default logging.properties sets log level for com.xyz.foo

2018-12-12 Thread Brian Burkhalter
Hi Daniel, Roger, +1 on the patch and Roger’s comment. Brian > On Dec 12, 2018, at 8:14 AM, Roger Riggs wrote: > > The change looks fine. > > The test addition is good, though it goes well beyond testing > that bug is no longer present.

[12] RFR for JDK-8214122: JDWP is broken on 32 bit Windows: transport library missing onLoad entry (was: [PATCH] Windows 32-bit DLL name decoration)

2018-12-12 Thread Alexey Ivanov
Hi all, I have updated the summary of JDK-8214122 and the subject accordingly. Could you please review the following fix? https://bugs.openjdk.java.net/browse/JDK-8214122 webrev: http://cr.openjdk.java.net/~aivanov/8214122/webrev.01/ *Root cause* jdwpTransport_OnLoad is exported as

Re: [PATCH] JDK-8214122 Prevent name mangling of jdwpTransport_OnLoad in Windows 32-bit DLL name decoration

2018-12-12 Thread Alexey Ivanov
Forgot to add the link: On 12/12/2018 16:10, Alexey Ivanov wrote: Ali has scanned the code base, he says, “couldn’t find (hopefully tbh) other occurrences of such mismatches.” http://mail.openjdk.java.net/pipermail/build-dev/2018-December/024358.html On 10/12/2018 10:45, Magnus Ihse Bursie

Re: RFR (trivial): 8211093: Default logging.properties sets log level for com.xyz.foo

2018-12-12 Thread Roger Riggs
Hi Daniel, The change looks fine. The test addition is good, though it goes well beyond testing that bug is no longer present. Thnaks, Roger On 12/12/2018 11:04 AM, Daniel Fuchs wrote: Hi, Please find below a fix for: 8211093: Default logging.properties sets log level for com.xyz.foo

Re: [PATCH] Windows 32-bit DLL name decoration

2018-12-12 Thread Alexey Ivanov
On 12/12/2018 16:52, Magnus Ihse Bursie wrote: On 2018-12-12 16:34, Alexey Ivanov wrote: On 12/12/2018 12:58, Magnus Ihse Bursie wrote: On 2018-12-12 12:35, Alan Bateman wrote: On 12/12/2018 11:14, Magnus Ihse Bursie wrote: : I searched the code for "jdwpTransport_On" to see of there

Re: 8214696: Module class should be filtered by core reflection

2018-12-12 Thread Mandy Chung
On 12/12/18 8:58 AM, Alan Bateman wrote: I'd like add java.lang.Module to list of classes that has its fields filtered from core reflection. As with previous addition in this area, this is another class where hacking its fields leads to integrity and security issues. The change is trivial:

Re: [12] RFR for JDK-8214122: JDWP is broken on 32 bit Windows: transport library missing onLoad entry

2018-12-12 Thread Magnus Ihse Bursie
On 2018-12-12 17:38, Alexey Ivanov wrote: Hi all, I have updated the summary of JDK-8214122 and the subject accordingly. Could you please review the following fix? https://bugs.openjdk.java.net/browse/JDK-8214122 webrev: http://cr.openjdk.java.net/~aivanov/8214122/webrev.01/ Looks good to

Re: RFR: 8215281: Use String.isEmpty() where applicable in java.base

2018-12-12 Thread Daniel Fuchs
Hi Claes, It might read even better if things like: +resultString = !specarg.isEmpty() ? specarg.intern(): opt; were changed into: +resultString = specarg.isEmpty() ? opt : specarg.intern(); best regards, -- daniel On 12/12/2018 16:53, Claes Redestad wrote: Hi, please

Re: RFR: 8215281: Use String.isEmpty() where applicable in java.base

2018-12-12 Thread Claes Redestad
On 2018-12-12 17:54, Daniel Fuchs wrote: Hi Claes, It might read even better if things like: +    resultString = !specarg.isEmpty() ? specarg.intern(): opt; were changed into: +    resultString = specarg.isEmpty() ? opt : specarg.intern(); best regards, I only found this

Re: [PATCH] Windows 32-bit DLL name decoration

2018-12-12 Thread Alexey Ivanov
On 12/12/2018 12:58, Magnus Ihse Bursie wrote: On 2018-12-12 12:35, Alan Bateman wrote: On 12/12/2018 11:14, Magnus Ihse Bursie wrote: : I searched the code for "jdwpTransport_On" to see of there was any corresponding OnUnload function (or other), but could not find any. That's right,

Re: [PATCH] JDK-8214122 Prevent name mangling of jdwpTransport_OnLoad in Windows 32-bit DLL name decoration

2018-12-12 Thread Alexey Ivanov
Ali has scanned the code base, he says, “couldn’t find (hopefully tbh) other occurrences of such mismatches.” On 10/12/2018 10:45, Magnus Ihse Bursie wrote: It might be worthwhile to scan the entire code base for JNICALL and verify that we have no more mismatches. In general, JNICALL should be

Re: RFR: JDK-8215217: OpenJDK Source Has Too Many Swear Words

2018-12-12 Thread mark . reinhold
2018/12/11 16:38:32 -0800, stuart.ma...@oracle.com: > Adam, > > Starting from your patch, I've removed changes relating to "crap" and "damn" > and > the changes to upstream jszip.js. This leaves the patches appended below. The > SoftChannel.java change is most likely a typo that should be

RFR JDK-8215300: additional changes to constants API

2018-12-12 Thread Vicente Romero
Hi, Please review some final changes to the constants API. The changes should make the API clearer and more precise and were recommended in the CSR [3] Thanks, Vicente [1] (jira issue) https://bugs.openjdk.java.net/browse/JDK-8215300 [2] (webrev)

RFR (trivial): 8211093: Default logging.properties sets log level for com.xyz.foo

2018-12-12 Thread Daniel Fuchs
Hi, Please find below a fix for: 8211093: Default logging.properties sets log level for com.xyz.foo https://bugs.openjdk.java.net/browse/JDK-8211093 Webrev: http://cr.openjdk.java.net/~dfuchs/webrev_8211093/webrev.00 best regards, -- daniel

Re: [PATCH] Windows 32-bit DLL name decoration

2018-12-12 Thread Alexey Ivanov
Hi Simon, I'd like to clarify some points… Please see my comments inline: On 11/12/2018 17:16, Alexey Ivanov wrote: Hi Simon, Thank you for your comments. The updated webrev: http://cr.openjdk.java.net/~aivanov/8214122/webrev.01/ Indeed, it looks much cleaner. Regards, Alexey On

Re: [PATCH] JDK-8214122 Prevent name mangling of jdwpTransport_OnLoad in Windows 32-bit DLL name decoration

2018-12-12 Thread Alexey Ivanov
Thank you, Ali, for doing this! On 10/12/2018 21:13, Ali İnce wrote: Hi Alexey, I’ve searched for |GetProcAddress| usages across source code and couldn’t find (hopefully tbh) other occurrences of such mismatches. Regards, Ali

Re: [PATCH] Windows 32-bit DLL name decoration

2018-12-12 Thread Magnus Ihse Bursie
On 2018-12-12 16:34, Alexey Ivanov wrote: On 12/12/2018 12:58, Magnus Ihse Bursie wrote: On 2018-12-12 12:35, Alan Bateman wrote: On 12/12/2018 11:14, Magnus Ihse Bursie wrote: : I searched the code for "jdwpTransport_On" to see of there was any corresponding OnUnload function (or

Re: 8214696: Module class should be filtered by core reflection

2018-12-12 Thread Lance Andersen
Looks good Alan > On Dec 12, 2018, at 11:58 AM, Alan Bateman wrote: > > I'd like add java.lang.Module to list of classes that has its fields filtered > from core reflection. As with previous addition in this area, this is another > class where hacking its fields leads to integrity and security

12 RFR (XS) 8215301: Module-summary page is unreadably wide

2018-12-12 Thread mark . reinhold
Bug: https://bugs.openjdk.java.net/browse/JDK-8215301 Webrev: https://cr.openjdk.java.net/~mr/rev/8215301/ The `make generate-summary` target produces a handy tabular summary of all of the modules in a JDK build into `$BUILD/images/gengraphs/module-summary.html`. Several JDK modules (e.g.,

Re: [PATCH] Windows 32-bit DLL name decoration

2018-12-12 Thread Alexey Ivanov
Hi Bob, Thank you for the pointers on how the same situation is handled in Hotspot for *_OnLoad* functions. At this time, we're dealing with one symbol only. If other symbols are added in the future, we should definitely use this approach. Regards, Alexey On 11/12/2018 17:22, Bob Vandette

Re: RFR: JDK-8066619: String(byte[],int,int,int) in String has been deprecated in Manifest and Attributes

2018-12-12 Thread Roger Riggs
Hi Phillip, Sorry, got busy... Can you rebase the patch to the current repo, it did not apply cleanly. I know you are focused on removing the deprecation, but a few localized improvements would be good. In Attributes.java : 346-337, it uses StringBuffer, please change it to StringBuilder.

RFR: 8215281: Use String.isEmpty() where applicable in java.base

2018-12-12 Thread Claes Redestad
Hi, please review this patch to use String.isEmpty when applicable: Webrev: http://cr.openjdk.java.net/~redestad/8215281/jdk.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-8215281 Why? - It reads better :-) - Better startup/warmup due fewer method invocations - Better peak performance:

Re: RFR: 8215281: Use String.isEmpty() where applicable in java.base

2018-12-12 Thread Alan Bateman
On 12/12/2018 16:53, Claes Redestad wrote: Hi, please review this patch to use String.isEmpty when applicable: Webrev: http://cr.openjdk.java.net/~redestad/8215281/jdk.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-8215281 Why? - It reads better :-) - Better startup/warmup due fewer method

RFR: 8215000: tools/launcher/JliLaunchTest.java fails on Windows

2018-12-12 Thread Henry Jen
Hi, Can I get a review of following patch? Looks like the assumption test jdk will be in PATH is simply not true, jtreg doesn’t do that. Also, this patch make sure the JDK to be tested is first in the search path. Cheers, Henry diff -r 241b8151b6b6 test/jdk/tools/launcher/JliLaunchTest.java

8214696: Module class should be filtered by core reflection

2018-12-12 Thread Alan Bateman
I'd like add java.lang.Module to list of classes that has its fields filtered from core reflection. As with previous addition in this area, this is another class where hacking its fields leads to integrity and security issues. The change is trivial:   

Re: RFR: 8215281: Use String.isEmpty() where applicable in java.base

2018-12-12 Thread Claes Redestad
On 2018-12-12 17:56, Alan Bateman wrote: In Checks.java, the parameter change from CharSequence to String means that "cs" needs to be renamed. Changed to 'str' /Claes

Re: JDK-8210280 - Unnecessary reallocation when invoking HashMap.putAll()

2018-12-12 Thread Martin Buchholz
Software is hard. I found myself removing the remaining style changes to be able to review the changes. (We're going to have to disagree about the value of curlies). Here's my reduction: --- src/main/java/util/HashMap.java 11 Nov 2018 16:27:28 - 1.9 +++ src/main/java/util/HashMap.java 12 Dec

Re: RFR 8215309 : Convert package.html files to package-info.java files

2018-12-12 Thread Lance Andersen
Hi Roger, I went through the changes and look OK as nothing jumped out. > On Dec 12, 2018, at 2:03 PM, Roger Riggs wrote: > > Please review a conversion of some package.html files to package-info.java > files. > This bunch targets packages: > > package com.sun.rowset.providers; >

Re: RFR 8215309 : Convert package.html files to package-info.java files

2018-12-12 Thread joe darcy
Hi Roger, From a quick skim, the changes looked fine. Thanks, -Joe On 12/12/2018 11:03 AM, Roger Riggs wrote: Please review a conversion of some package.html files to package-info.java files. This bunch targets packages:    package com.sun.rowset.providers;    package java.rmi;    package

Re: RFR: 8215000: tools/launcher/JliLaunchTest.java fails on Windows

2018-12-12 Thread Henry Jen
Duh, should be v==null. Thanks for catching it. Cheers, Henry > On Dec 12, 2018, at 10:50 AM, Mandy Chung wrote: > > Brent is right since k is the given key and non-null. Although it does not > cause any issue as it only adds an empty element in the path, we should fix > it in this patch. >

Re: RFR: 8215000: tools/launcher/JliLaunchTest.java fails on Windows

2018-12-12 Thread Brent Christian
Hi, Shouldn't the lambdas be checking for v == null, rather than k == null? -Brent On 12/12/18 9:36 AM, Henry Jen wrote: Hi, Can I get a review of following patch? Looks like the assumption test jdk will be in PATH is simply not true, jtreg doesn’t do that. Also, this patch make sure the

Re: 12 RFR (XS) 8215301: Module-summary page is unreadably wide

2018-12-12 Thread Mandy Chung
Looks good. This makes the list of providers very clear.   Thanks for making this change. Mandy On 12/12/18 9:27 AM, mark.reinh...@oracle.com wrote: Bug: https://bugs.openjdk.java.net/browse/JDK-8215301 Webrev: https://cr.openjdk.java.net/~mr/rev/8215301/ The `make generate-summary` target

Re: RFR: 8215000: tools/launcher/JliLaunchTest.java fails on Windows

2018-12-12 Thread Mandy Chung
Brent is right since k is the given key and non-null.  Although it does not cause any issue as it only adds an empty element in the path, we should fix it in this patch. Mandy On 12/12/18 10:30 AM, Brent Christian wrote: Hi, Shouldn't the lambdas be checking for v == null, rather than k ==

Re: RFR: 8170769 Provide a simple hexdump facility for binary data

2018-12-12 Thread Vincent Ryan
FYI I’ve updated the webrev/javadoc with latest edits including the 2 new ByteBuffer methods: http://cr.openjdk.java.net/~vinnie/8170769/webrev.09/

Re: RFR 8215309 : Convert package.html files to package-info.java files

2018-12-12 Thread Roger Riggs
Thanks Joe, Lance. On 12/12/2018 02:44 PM, Lance Andersen wrote: Hi Roger, I went through the changes and look OK as nothing jumped out. On Dec 12, 2018, at 2:03 PM, Roger Riggs > wrote: Please review a conversion of some package.html files to

Re: JDK-8210280 - Unnecessary reallocation when invoking HashMap.putAll()

2018-12-12 Thread Martin Buchholz
When hacking on HashMap, one always needs to keep LinkedHashMap in the back of one's mind. If you use removeEldestEntry then the occupancy can never get beyond some limit. It may be weird to have elements that are part of a putAll being removed during the call to putAll. There's already a call

Re: 12 RFR (XS) 8215301: Module-summary page is unreadably wide

2018-12-12 Thread David Holmes
On 13/12/2018 3:27 am, mark.reinh...@oracle.com wrote: Bug: https://bugs.openjdk.java.net/browse/JDK-8215301 Webrev: https://cr.openjdk.java.net/~mr/rev/8215301/ The `make generate-summary` target produces a handy tabular summary of all of the modules in a JDK build into

RFR 8215309 : Convert package.html files to package-info.java files

2018-12-12 Thread Roger Riggs
Please review a conversion of some package.html files to package-info.java files. This bunch targets packages: package com.sun.rowset.providers; package java.rmi; package java.rmi.activation; package java.rmi.dgc; package java.rmi.registry; package java.rmi.server; package

RE: OpenJDK fails to build with GCC when the #include inside zip.cpp comes from a non-sysroot path

2018-12-12 Thread Patrick Zhang
Ping... Apply for a sponsor for this simple patch. I doubt if I could have permission to file the issue/bug report anywhere, could anyone kindly give a guidance? Thanks. Regards Patrick -Original Message- From: core-libs-dev On Behalf Of Patrick Zhang Sent: Thursday, December 6,

Re: 8214696: Module class should be filtered by core reflection

2018-12-12 Thread Sundararajan Athijegannathan
Looks good -Sundar On 12/12/18, 10:28 PM, Alan Bateman wrote: I'd like add java.lang.Module to list of classes that has its fields filtered from core reflection. As with previous addition in this area, this is another class where hacking its fields leads to integrity and security issues. The

Re: RFR(s): JDK-8214687 Optimize Collections.nCopies().hashCode()

2018-12-12 Thread Zheka Kozlov
OK, this is a fixed version: @Override public void forEach(Consumer action) { Objects.requireNonNull(action); final int n = this.n; final E e = this.element; for (int i = 0; i < n; i++) { action.accept(e); } } Tagir, can you add this to your patch? I signed the OCA.

Re: RFR(s): JDK-8214687 Optimize Collections.nCopies().hashCode()

2018-12-12 Thread Tagir Valeev
Hello, Zheka! I'm not sure whether it's possible to commit a patch which is partially contributed by another person. Probably you should submit it separately? Also for complete patch a testcase would be necessary. With best regards, Tagir Valeev. On Thu, Dec 13, 2018 at 11:48 AM Zheka Kozlov

Re: RFR: 8211752: JNU_ThrowIOExceptionWithLastErrorAndPath - enhance some IOExceptions with path causing the issue

2018-12-12 Thread Alan Bateman
On 11/12/2018 20:00, Sean Mullan wrote: These exceptions are generated from a very low level part of the native JDK Windows or Unix FileSystem implementations. That is a concern. The previous usages of this property were more focused and confined to smaller parts of the code resulting in

Re: RFR: JDK-8215217: OpenJDK Source Has Too Many Swear Words

2018-12-12 Thread Andrew Dinn
On 11/12/2018 21:52, mark.reinh...@oracle.com wrote: > I can certainly see removing the f-word, and other words of a sexual > nature. Those are clearly inappropriate. > > Removing lesser words, and continuing to police their use henceforth, > strikes me as overkill. > > What do other Committers

Re: Proposal: Use new JDK_EXPORT decorator instead of JNIEXPORT

2018-12-12 Thread Magnus Ihse Bursie
On 2018-12-12 09:40, David Holmes wrote: On 12/12/2018 5:44 pm, Volker Simonis wrote: On Tue, Dec 11, 2018 at 11:47 PM David Holmes wrote: On 12/12/2018 12:34 am, Magnus Ihse Bursie wrote: On 2018-12-11 00:23, David Holmes wrote: Hi Magnus, On 10/12/2018 11:19 pm, Magnus Ihse Bursie

Re: Proposal: Use new JDK_EXPORT decorator instead of JNIEXPORT

2018-12-12 Thread Magnus Ihse Bursie
On 2018-12-11 23:47, David Holmes wrote: On 12/12/2018 12:34 am, Magnus Ihse Bursie wrote: On 2018-12-11 00:23, David Holmes wrote: Hi Magnus, On 10/12/2018 11:19 pm, Magnus Ihse Bursie wrote: I propose that we introduce a new define, available to all JDK native files (Hotspot

Re: [PATCH] Windows 32-bit DLL name decoration

2018-12-12 Thread Magnus Ihse Bursie
On 2018-12-11 18:22, Bob Vandette wrote: Hotspot has had support for decorated and non-decorated names for the JNI_OnLoad functions. Perhaps you should just follow the same procedure for the debug library. #define JNI_ONLOAD_SYMBOLS {"_JNI_OnLoad@8", "JNI_OnLoad"} #define

Re: RFR: JDK-8215217: OpenJDK Source Has Too Many Swear Words

2018-12-12 Thread Andrew Haley
On 12/11/18 9:52 PM, mark.reinh...@oracle.com wrote: > I can certainly see removing the f-word, and other words of a sexual > nature. Those are clearly inappropriate. Fair enough, but we shouldn't touch imported sources, as Joe Darcy pointed out. Let the rudeness be fixed at source. > Removing

Re: RFR: JDK-8215217: OpenJDK Source Has Too Many Swear Words

2018-12-12 Thread Andrew Haley
On 12/11/18 3:03 PM, Adam Farley8 wrote: > I've spotted 12 instances of swear words in OpenJDK source comments, and > it seems appropriate to remove them. > > Bug: https://bugs.openjdk.java.net/browse/JDK-8215217 > > I've created a webrev and attached to the bug. > > Also, I've mentioned in

Re: [PATCH] Windows 32-bit DLL name decoration

2018-12-12 Thread Magnus Ihse Bursie
On 2018-12-11 18:16, Alexey Ivanov wrote: Hi Simon, Thank you for your comments. The updated webrev: http://cr.openjdk.java.net/~aivanov/8214122/webrev.01/ Indeed, it looks much cleaner. Yes! This seems like the correct fix. Ship it! :) /Magnus Regards, Alexey On 11/12/2018 16:43,

Re: RFR: JDK-8215217: OpenJDK Source Has Too Many Swear Words

2018-12-12 Thread Remi Forax
- Mail original - > De: "Andrew Haley" > À: "Adam Farley8" , "core-libs-dev" > > Envoyé: Mercredi 12 Décembre 2018 10:50:52 > Objet: Re: RFR: JDK-8215217: OpenJDK Source Has Too Many Swear Words > On 12/11/18 3:03 PM, Adam Farley8 wrote: > >> I've spotted 12 instances of swear

Re: Proposal: Use new JDK_EXPORT decorator instead of JNIEXPORT

2018-12-12 Thread David Holmes
On 12/12/2018 5:44 pm, Volker Simonis wrote: On Tue, Dec 11, 2018 at 11:47 PM David Holmes wrote: On 12/12/2018 12:34 am, Magnus Ihse Bursie wrote: On 2018-12-11 00:23, David Holmes wrote: Hi Magnus, On 10/12/2018 11:19 pm, Magnus Ihse Bursie wrote: I propose that we introduce a new

Re: Proposal: Use new JDK_EXPORT decorator instead of JNIEXPORT

2018-12-12 Thread David Holmes
Okay I went away and did some homework ... Let me back up a bit and see if I have the evolution of this correctly. The native implementation of Java methods should be declared and defined with JNIEXPORT and JNICALL. JNIEXPORT controls the export visibility so they can be linked. JNICALL

Re: [PATCH] Windows 32-bit DLL name decoration

2018-12-12 Thread Alan Bateman
On 12/12/2018 11:14, Magnus Ihse Bursie wrote: : I searched the code for "jdwpTransport_On" to see of there was any corresponding OnUnload function (or other), but could not find any. That's right, an unload wasn't needed when that SPI was originally added but could be added in the event that

Re: RFR: 8170769 Provide a simple hexdump facility for binary data

2018-12-12 Thread Vincent Ryan
Thanks for your proposal. Comments below. > On 12 Dec 2018, at 02:35, Stuart Marks wrote: > > > > On 12/11/18 1:21 PM, Vincent Ryan wrote: >> My preference is for PrintStream rather than Writer, for the same reason as >> Roger: it’s more convenient >> for handling System.out. Does that

Re: [PATCH] Windows 32-bit DLL name decoration

2018-12-12 Thread Magnus Ihse Bursie
On 2018-12-12 12:35, Alan Bateman wrote: On 12/12/2018 11:14, Magnus Ihse Bursie wrote: : I searched the code for "jdwpTransport_On" to see of there was any corresponding OnUnload function (or other), but could not find any. That's right, an unload wasn't needed when that SPI was

Re: JDK-8210280 - Unnecessary reallocation when invoking HashMap.putAll()

2018-12-12 Thread Michal Vala
Hi Martin, thanks for review and sponsorship. All fixed, details inlined. new webrev: http://cr.openjdk.java.net/~mvala/jdk/jdk/JDK-8210280/webrev.01/ On 12/12/18 3:55 AM, Martin Buchholz wrote: Hi Michal, pleased to meet you. I'll be your sponsor. The test will need a legal header,

Re: RFR: 8213754: PPC64: Add Intrinsics for isDigit/isLowerCase/isUpperCase/isWhitespace

2018-12-12 Thread Gustavo Romero
Hi Martin, On 12/12/2018 05:10 AM, Doerr, Martin wrote: thanks for your improvements. You can also remove the trailing \t from the opto assembly. Note that there's no need to re-push newer version to jdk/submit when only PPC files were changed. jdk/submit doesn't look at them. Got it.

Re: JDK-8210280 - Unnecessary reallocation when invoking HashMap.putAll()

2018-12-12 Thread Michal Vala
thank you Roger, I've added jmh benchmark. see new webrev in reply to Martin's review. On 12/12/18 4:45 AM, Roger Riggs wrote: Hi, fyi, jmh tests appropriate for JDK apis are located in the test/micro/... directories. The benchmarks are built with:   % make build-microbenchmark The