Re: [RFR] 8205525 : Improve exception messages during manifest parsing of jar archives

Hi Max, thanks for adding yourself as a reviewer. I set the CSR ( https://bugs.openjdk.java.net/browse/JDK-8207768 ) to proposed. Best regards, Matthias > > Message: 2 > Date: Tue, 4 Sep 2018 21:31:58 +0800 > From: Weijun Wang > To: "Baesken, Matthias" > Cc: "security-...@openjdk.java.n

Re: RFR: 8139965 - Hang seen when using com.sun.jndi.ldap.search.replyQueueSize

Hi Rob, That looks better but I believe that: 1. closed should be volatile since it's read from outside synchronized block 2. it seems there might be a race where the last response received could be dropped, if the connection is closed just after it's been pulled from the queue. So I

Re: RFR 8210318: idea.sh script doesn't work on Mac

Here's the modified webrev - as suggested, I replaced /tmp/replacement with $1.tmp http://cr.openjdk.java.net/~mcimadamore/8210318_v2/ Thanks Maurizio On 04/09/18 18:50, Erik Joelsson wrote: Hello, $TARGET was just pseudo code. In your case it's $1.tmp. /Erik On 2018-09-04 10:34, Maurizi

Re: RFR 8210318: idea.sh script doesn't work on Mac

On 2018-09-05 13:42, Maurizio Cimadamore wrote: Here's the modified webrev - as suggested, I replaced /tmp/replacement with $1.tmp http://cr.openjdk.java.net/~mcimadamore/8210318_v2/ Looks good to me. /Magnus Thanks Maurizio On 04/09/18 18:50, Erik Joelsson wrote: Hello, $TARGET was j

RFR: JDK-8200609 Proper fix for mapfile removal for libjsig

When I removed (mostly) all mapfiles for the JDK libraries, building of libjsig started failing on Solaris, and that part was backed out. Here's a new, and improved solution to remove the mapfile for libjsig, even on Solaris. Bug: https://bugs.openjdk.java.net/browse/JDK-8200609 WebRev: http

RFR: JDK-8059019 sdp.conf.template should move from unix to solaris

The file sdp.conf.template is copied only when building for solaris, but it resides in the unix source directory. Bug: https://bugs.openjdk.java.net/browse/JDK-8059019 Patch inline: diff --git a/make/copy/Copy-java.base.gmk b/make/copy/Copy-java.base.gmk --- a/make/copy/Copy-java.base.gmk +++ b/

RFR: 8210416: [linux] Poor StrictMath performance due to non-optimized compilation

Hi, Cross-posting this review-thread on core-libs-dev and build-dev as this is a build change, but affects fdlibm which is core-libs. With JDK-8170153 optimization for fdlibm code has been turned on for ppc64 s390 and aarch64. This patch intends to turn it on on all arches on Linux. I've not obse

Re: RFR: JDK-8059019 sdp.conf.template should move from unix to solaris

On 05/09/2018 13:58, Magnus Ihse Bursie wrote: The file sdp.conf.template is copied only when building for solaris, but it resides in the unix source directory. SDP is legacy now and we should probably think about removing it, esp. with the rsocket support in the works. In the mean time, as th

Re: [12] RFR: 8210142: java.util.Calendar.clone() doesn't respect sharedZone flag

Hi Naoto, That sounds like a test issue.  I would not expect a cloned Calendar or TimeZone to have a different hashCode. None of the fields involved in the hashCode/equals should be different. (Or I'm missing something about them). Thanks, Roger On 9/4/18 5:19 PM, Naoto Sato wrote: Hi Roge

RE: [RFR] 8205525 : Improve exception messages during manifest parsing of jar archives

Hi Matthias, I have reviewed your change, +1 I also reviewed the CSR. Best regards Christoph > -Original Message- > From: Baesken, Matthias > Sent: Mittwoch, 5. September 2018 10:07 > To: security-...@openjdk.java.net; Weijun Wang > ; core-libs-dev@openjdk.java.net > Cc: Langer, Christo

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 tri

Question about libjava/childproc.c

Hi all, I just saw that in childproc.c, between fork and exec, in the child process, we attempt to close all file descriptors. Which is fine and good practice. See function "closeDecriptors()" in libjava/childproc.c. That function attempts to read /proc/fd to get all open file descriptors and th

Re: 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

On 05/09/2018 16:38, Xueming Shen wrote: 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()..." w

Re: 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

+1 > On Sep 5, 2018, at 12:38 PM, Xueming Shen wrote: > > 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

Re: Question about libjava/childproc.c

On 05/09/2018 16:45, Thomas Stüfe wrote: : My question would be, could we not - instead of straight away closing the file descriptor - set them all to FD_CLOEXEC instead? This comes up periodically but even if we do that then we still need this code to catch the places where FD_CLOEXEC isn't s

Re: Question about libjava/childproc.c

Hi Alan, On Wed, Sep 5, 2018 at 6:06 PM, Alan Bateman wrote: > On 05/09/2018 16:45, Thomas Stüfe wrote: >> >> : >> >> My question would be, could we not - instead of straight away closing >> the file descriptor - set them all to FD_CLOEXEC instead? >> > This comes up periodically but even if we d

Re: Question about libjava/childproc.c

Alan: Thomas seems to be suggesting setting the FD_CLOEXEC flag after fork but before exec, which is a slightly different idea. Thomas: This is an interesting idea. Historically the usual strategy was to close all the file descriptors explicitly, perhaps before FD_CLOEXEC was something we could r

Re: Question about libjava/childproc.c

On Wed, Sep 5, 2018 at 6:16 PM, Martin Buchholz wrote: > Alan: Thomas seems to be suggesting setting the FD_CLOEXEC flag after fork > but before exec, which is a slightly different idea. > > Thomas: This is an interesting idea. Historically the usual strategy was to > close all the file descripto

Re: Question about libjava/childproc.c

On Wed, Sep 5, 2018 at 9:23 AM, Thomas Stüfe wrote: > > Since all this happens between vfork and exec we cannot do any decent > error handling here. Even what little we do is way outside the allowed > spec for vfork(). > Well we do have a fallback if closeDescriptors fails. But on closer inspec

Re: RFR 8210318: idea.sh script doesn't work on Mac

Looks good, thanks! /Erik On 2018-09-05 04:42, Maurizio Cimadamore wrote: Here's the modified webrev - as suggested, I replaced /tmp/replacement with $1.tmp http://cr.openjdk.java.net/~mcimadamore/8210318_v2/ Thanks Maurizio On 04/09/18 18:50, Erik Joelsson wrote: Hello, $TARGET was jus

Re: RFR: JDK-8200609 Proper fix for mapfile removal for libjsig

Hello, Looks good to me. /Erik On 2018-09-05 05:11, Magnus Ihse Bursie wrote: When I removed (mostly) all mapfiles for the JDK libraries, building of libjsig started failing on Solaris, and that part was backed out. Here's a new, and improved solution to remove the mapfile for libjsig, eve

Re: [PATCH] Simplification of TreeMap

Hi Michael, thanks for pointing out this serialization concern, I didn't think about it at all. I've wrote a simple test for serialization of patched TreeMap and it works without errors for both no-args constructor and constructor with comparator: public class TreeMapSerialization { public

Re: Question about libjava/childproc.c

On Wed, Sep 5, 2018 at 6:43 PM, Martin Buchholz wrote: > > > On Wed, Sep 5, 2018 at 9:23 AM, Thomas Stüfe > wrote: >> >> >> Since all this happens between vfork and exec we cannot do any decent >> error handling here. Even what little we do is way outside the allowed >> spec for vfork(). > > > We

Re: [PATCH] Simplification of TreeMap

Сергей, they say "If it ain't broke, don't fix it". Your implementation is indeed more maintainable, but TreeMap has been rather stable and is unlikely to see much maintenance! (unless value types makes it compelling). One of many problems with serialization is that cross-version compatibility i

Re: RFR: 8210416: [linux] Poor StrictMath performance due to non-optimized compilation

Hello, On 9/5/2018 6:12 AM, Severin Gehwolf wrote: Hi, Cross-posting this review-thread on core-libs-dev and build-dev as this is a build change, but affects fdlibm which is core-libs. With JDK-8170153 optimization for fdlibm code has been turned on for ppc64 s390 and aarch64. This patch inte

Re: [PATCH] Simplification of TreeMap

Hi Sergey, Michael might correct me if I’ve missed something, but problem with your test case is that you’re serializing already patched version. That makes sense if you want to test current behavior. However the case you truly want to test is how your patched TreeMap deserializes pre-patched Tree

Re: RFR: 8210416: [linux] Poor StrictMath performance due to non-optimized compilation

Hello, On 09/05/2018 04:15 PM, joe darcy wrote: Hello, On 9/5/2018 6:12 AM, Severin Gehwolf wrote: Hi, Cross-posting this review-thread on core-libs-dev and build-dev as this is a build change, but affects fdlibm which is core-libs. With JDK-8170153 optimization for fdlibm code has been tur

Re: RFR - JDK-8200434 - String::align, String::indent (code review)

Hi Jim, Overall it looks fine. Some quibbles on the wording and the test. The spec for the align() and align(n) methods in String.java show a possibly misleading inconsistency. The first line says: Removes vertical and horizontal white space margins. But later align() says: Trailing

Re: [12] RFR: 8210142: java.util.Calendar.clone() doesn't respect sharedZone flag

Hi Roger, I updated the fix to share the zone only if the sharedZone is true. Here is the updated webrev: http://cr.openjdk.java.net/~naoto/8210142/webrev.01/ Naoto On 9/5/18 7:07 AM, Roger Riggs wrote: Hi Naoto, That sounds like a test issue.  I would not expect a cloned Calendar or Time

Re: RFR(L/M) : 8210112 : remove jdk.testlibrary.ProcessTools

Hi JC, thanks for reviewing this! I agree w/ both your comments and have updated the code very similarly to your suggestion. I've also noticed that j.t.l.p.OutputAnalyzer::shouldMatchByLine method family is a bit different from other should* (and strange), besides checking that the lines match

Re: RFR: 8210416: [linux] Poor StrictMath performance due to non-optimized compilation

Hi Severin, Might as well raise this here too as it's really a build philosophy issue. Shouldn't flags like -ffp-contract=off (and the existing AIX -qfloat=nomaf) be toolchain specific rather than platform specific? Thanks, David On 5/09/2018 11:12 PM, Severin Gehwolf wrote: Hi, Cross-post

Re: RFR(L/M) : 8210112 : remove jdk.testlibrary.ProcessTools

Hi Igor, I like this much better! A few more comments: - http://cr.openjdk.java.net/~iignatyev//8210112/webrev.0-1/test/jdk/lib/testlibrary/OutputAnalyzerTest.java.udiff.html -> If the shouldMatch call fails, it throws an exception, why not just let that fail test, why are you catching and then

Re: RFR: JDK-8200609 Proper fix for mapfile removal for libjsig

Hi Magnus, On 5/09/2018 10:11 PM, Magnus Ihse Bursie wrote: When I removed (mostly) all mapfiles for the JDK libraries, building of libjsig started failing on Solaris, and that part was backed out. Here's a new, and improved solution to remove the mapfile for libjsig, even on Solaris. Bug:

Re: RFR: JDK-8200609 Proper fix for mapfile removal for libjsig

> 6 sep. 2018 kl. 00:00 skrev David Holmes : > > Hi Magnus, > >> On 5/09/2018 10:11 PM, Magnus Ihse Bursie wrote: >> When I removed (mostly) all mapfiles for the JDK libraries, building of >> libjsig started failing on Solaris, and that part was backed out. >> Here's a new, and improved soluti

Re: RFR(L/M) : 8210112 : remove jdk.testlibrary.ProcessTools

Hi JC, > On Sep 5, 2018, at 2:59 PM, JC Beyler wrote: > > Hi Igor, > > I like this much better! A few more comments: > > - > http://cr.openjdk.java.net/~iignatyev//8210112/webrev.0-1/test/jdk/lib/testlibrary/OutputAnalyzerTest.java.udiff.html > >

Re: RFR: JDK-8200609 Proper fix for mapfile removal for libjsig

On 6/09/2018 8:13 AM, Magnus Ihse Bursie wrote: 6 sep. 2018 kl. 00:00 skrev David Holmes : Hi Magnus, On 5/09/2018 10:11 PM, Magnus Ihse Bursie wrote: When I removed (mostly) all mapfiles for the JDK libraries, building of libjsig started failing on Solaris, and that part was backed out. He