Re: RFR (S): JDK-8222778: Packaging Tool (JEP 343) on Linux/AArch64

2019-07-16 Thread Dmitry Chuyko
Alexander, thanks for having a look, On 7/17/19 12:30 AM, Alexander Matveev wrote: Hi Dmitry, http://cr.openjdk.java.net/~dchuyko/8222778/webrev.00/src/jdk.jpackage/unix/native/libapplauncher/PosixPlatform.cpp.frames.html Why code between lines 215 and 219 was disabled? Not sure what it

Re: RFR (S): JDK-8222778: Packaging Tool (JEP 343) on Linux/AArch64

2019-07-16 Thread Alexander Matveev
Hi Dmitry, http://cr.openjdk.java.net/~dchuyko/8222778/webrev.00/src/jdk.jpackage/unix/native/libapplauncher/PosixPlatform.cpp.frames.html Why code between lines 215 and 219 was disabled? Not sure what it tries to do, if it tries to guarantee NULL termination we should probably keep it or

Re: RFR: 8227642: [TESTBUG] Make docker tests podman compatible

2019-07-16 Thread mikhailo . seledtsov
On 7/16/19 1:32 PM, Igor Ignatyev wrote: Hi Misha, I understand that it doesn't alter the host system. my concern is that we move problem of host-configuration into tests. let's say tomorrow a new container engine will require something from /usr/local/sbin, or /usr/local/Cellar/docker/bin

Re: RFR: 8227642: [TESTBUG] Make docker tests podman compatible

2019-07-16 Thread Severin Gehwolf
On Tue, 2019-07-16 at 14:11 -0700, Jonathan Gibbons wrote: > Severin, > > This might be a reasonable update to jtreg, to allow /usr/sbin on the path > on Unix-like systems. The intent of jtreg is to protect tests from random > crufty stuff on the PATH, and /usr/sbin is not in that category. > >

Re: RFR: 8227642: [TESTBUG] Make docker tests podman compatible

2019-07-16 Thread Jonathan Gibbons
Severin, This might be a reasonable update to jtreg, to allow /usr/sbin on the path on Unix-like systems.  The intent of jtreg is to protect tests from random crufty stuff on the PATH, and /usr/sbin is not in that category. I've created CODETOOLS-7902505: Consider allowing /usr/sbin on $PATH

Re: RFR: 8227642: [TESTBUG] Make docker tests podman compatible

2019-07-16 Thread Severin Gehwolf
Hi Igor, I understand the concern and I guess I could remove it and locally install a wrapper in /bin or /usr/bin for podman which adds /usr/sbin to the path. On the other hand... This seems to be an issue of code being run through jtreg. Looking at the jtr files I see this:

Re: RFR: 8227642: [TESTBUG] Make docker tests podman compatible

2019-07-16 Thread Igor Ignatyev
Hi Misha, I understand that it doesn't alter the host system. my concern is that we move problem of host-configuration into tests. let's say tomorrow a new container engine will require something from /usr/local/sbin, or /usr/local/Cellar/docker/bin on another OS, or, god forbid, C:\Program

Re: RFR: 8227642: [TESTBUG] Make docker tests podman compatible

2019-07-16 Thread mikhailo . seledtsov
Hi Igor,    In both cases the environment variable is set for the Docker/Podman container process, not the host system. This will not affect the host system in any way. The docker process has its own namespace for environment variables. Does this alleviate your concerns? Thank you, Misha

RFR (S): JDK-8222778: Packaging Tool (JEP 343) on Linux/AArch64

2019-07-16 Thread Dmitry Chuyko
Hello, Please review a small patch that mostly fixes jpackage test for Linux aarch64 and also arm,x86,power. It is prepared for 'JDK-8200758-branch' branch of open 'sandbox' repo. There are few parts: 1. LinuxPlatform.cpp and IniFile.cpp got small fixes for compiler warnings. 2.

Re: 8073213: javadoc of PushbackInputStream methods should specify NullPointerExceptions

2019-07-16 Thread Lance Andersen
Looks fine Brian > On Jul 16, 2019, at 3:45 PM, Brian Burkhalter > wrote: > > I changed … to {@code …} and @exception to @throws in [3]. The > delta versus the original version [1] is at [2]. > > Thanks, > > Brian > > [1] http://cr.openjdk.java.net/~bpb/8073213/webrev.00/ > [2]

Re: 8073213: javadoc of PushbackInputStream methods should specify NullPointerExceptions

2019-07-16 Thread Brian Burkhalter
I changed … to {@code …} and @exception to @throws in [3]. The delta versus the original version [1] is at [2]. Thanks, Brian [1] http://cr.openjdk.java.net/~bpb/8073213/webrev.00/ [2] http://cr.openjdk.java.net/~bpb/8073213/webrev.00-01/ [3] http://cr.openjdk.java.net/~bpb/8073213/webrev.01/

Re: 8131664: Javadoc for PrintStream is now incorrect

2019-07-16 Thread Lance Andersen
I think this is OK Brian > On Jul 16, 2019, at 3:16 PM, Brian Burkhalter > wrote: > > An updated version which includes the suggested change below is at [1] with a > diff versus the reviewed version .00 at [2]. > > Thanks, > > Brian > > [1]

Re: RFR [14/java.xml] 8176447: javax.xml.validation.Validator validates incorrectly on uniqueness constraint

2019-07-16 Thread Joe Wang
Thanks Lance! On 7/16/19 12:20 PM, Lance Andersen wrote: Looks OK Joe On Jul 16, 2019, at 12:34 PM, Joe Wang > wrote: Please review a fix for the uniqueness constraint. The fix is at line 403 where the variable for tracking matches is reset. Without the

Re: RFR [14/java.xml] 8176447: javax.xml.validation.Validator validates incorrectly on uniqueness constraint

2019-07-16 Thread Lance Andersen
Looks OK Joe > On Jul 16, 2019, at 12:34 PM, Joe Wang wrote: > > Please review a fix for the uniqueness constraint. The fix is at line 403 > where the variable for tracking matches is reset. Without the reset, > subsequent matches would have been skipped, thus not catching duplicate >

Re: 8131664: Javadoc for PrintStream is now incorrect

2019-07-16 Thread Brian Burkhalter
An updated version which includes the suggested change below is at [1] with a diff versus the reviewed version .00 at [2]. Thanks, Brian [1] http://cr.openjdk.java.net/~bpb/8131664/webrev.01/ [2] Version .01 vs. version .00 ---

DnsClient TCP socket timeout

2019-07-16 Thread Milan Mimica
Hello list Have you ever considered the problem reported here: https://stackoverflow.com/questions/14603553/dns-query-freezes-with-dnscontextfactory-in-java A DNS query gets stuck easily if read timeout is not set on the socket. The proposed solution is trivial. -- Milan Mimica

Re: RFR: 8227642: [TESTBUG] Make docker tests podman compatible

2019-07-16 Thread Igor Ignatyev
Hi Severin, I don't think that tests (or test libraries for that matter) should be responsible for setting correct PATH value, it should be a part of host configuration procedure (tests can/should check that all required bins are available though). in other words, I'd prefer if you remove

Re: 8073213: javadoc of PushbackInputStream methods should specify NullPointerExceptions

2019-07-16 Thread Lance Andersen
+1 > On Jul 16, 2019, at 12:08 PM, Brian Burkhalter > wrote: > > https://bugs.openjdk.java.net/browse/JDK-8073213 > > > Add javadoc of NPEs missing from two unread() methods; see below. > > It would not hurt in this class to also change … to

Re: RFR: 8227587: Add internal privileged System.loadLibrary

2019-07-16 Thread Mandy Chung
On 7/16/19 6:48 AM, Claes Redestad wrote: Hi, refactored to use a BootLoader::loadLibrary API that is only visible within the java.base module: http://cr.openjdk.java.net/~redestad/8227587/open.03/ Looks good. Nit:  in JavaLangAccess    321 void loadLibrary(Class klass, String

RFR [14/java.xml] 8176447: javax.xml.validation.Validator validates incorrectly on uniqueness constraint

2019-07-16 Thread Joe Wang
Please review a fix for the uniqueness constraint. The fix is at line 403 where the variable for tracking matches is reset. Without the reset, subsequent matches would have been skipped, thus not catching duplicate values. As the report's workaround indicated, this issue was fixed in Apache

8073213: javadoc of PushbackInputStream methods should specify NullPointerExceptions

2019-07-16 Thread Brian Burkhalter
https://bugs.openjdk.java.net/browse/JDK-8073213 Add javadoc of NPEs missing from two unread() methods; see below. It would not hurt in this class to also change … to {@code …} and @exception to @throws but that would clutter up the review.

RFR : 8227737: avoid implicit-function-declaration on AIX

2019-07-16 Thread Baesken, Matthias
Hello, please review the following AIX related change . It fixes a number of missing inclusions leading to implicit-function-declaration warnings when compiling with the recent xlc16 /xlclang . At various places in the native C coding in jdk, we miss header inclusions on AIX. This leads

Re: RFR: 8227587: Add internal privileged System.loadLibrary

2019-07-16 Thread Roger Riggs
+1 On 7/16/19 10:52 AM, Chris Hegarty wrote: On 16 Jul 2019, at 14:48, Claes Redestad wrote: Hi, refactored to use a BootLoader::loadLibrary API that is only visible within the java.base module: http://cr.openjdk.java.net/~redestad/8227587/open.03/ I think that this is good Claes.

Re: RFR: 8227587: Add internal privileged System.loadLibrary

2019-07-16 Thread Chris Hegarty
> On 16 Jul 2019, at 14:48, Claes Redestad wrote: > > Hi, > > refactored to use a BootLoader::loadLibrary API that is only visible > within the java.base module: > > http://cr.openjdk.java.net/~redestad/8227587/open.03/ I think that this is good Claes. Reviewed. -Chris.

Re: RFR: 8227587: Add internal privileged System.loadLibrary

2019-07-16 Thread Claes Redestad
Hi, refactored to use a BootLoader::loadLibrary API that is only visible within the java.base module: http://cr.openjdk.java.net/~redestad/8227587/open.03/ I've retained the bridge to ClassLoader::loadLibrary for performance, but without any magic or privilege escalation involved. Moving

Re: RFR: 8227642: [TESTBUG] Make docker tests podman compatible

2019-07-16 Thread Severin Gehwolf
Hi, I believe I still need a *R*eviewer for this. Any takers? Thanks, Severin On Fri, 2019-07-12 at 15:19 -0700, mikhailo.seledt...@oracle.com wrote: > Hi Severin, > >The change looks good to me. Thank you for adding support for Podman > container technology. > > Testing: I ran both