Re: RFR(L): 8031581: PPC64: Addons and fixes for AIX to pass the jdk regression tests

2014-01-15 Thread Staffan Larsen
Volker,

I’ve look at the following files:

src/share/native/sun/management/DiagnosticCommandImpl.c:
nit: “legel” - “legal” (two times)
In Java_sun_management_DiagnosticCommandImpl_getDiagnosticCommandInfo() if you 
allow dcmd_info_array to become NULL, then 
jmm_interface-GetDiagnosticCommandInfo() will throw an NPE and you need to 
check that.

src/solaris/native/sun/management/OperatingSystemImpl.c
No comments.

src/share/transport/socket/socketTransport.c
No comments.

src/share/classes/sun/tools/attach/META-INF/services/com.sun.tools.attach.spi.AttachProvider
No comments.


Thanks,
/Staffan



On 14 jan 2014, at 09:40, Volker Simonis volker.simo...@gmail.com wrote:

 Hi,
 
 could you please review the following changes for the ppc-aix-port 
 stage/stage-9 repositories (the changes are planned for integration into 
 ppc-aix-port/stage-9 and subsequent backporting to ppc-aix-port/stage):
 
 http://cr.openjdk.java.net/~simonis/webrevs/8031581/
 
 I've build and smoke tested without any problems on Linux/x86_64 and PPC64, 
 Windows/x86_64, MacOSX, Solaris/SPARC64 and AIX7PPC64.
 
 With these changes (and together with the changes from 8028537: PPC64: 
 Updated jdk/test scripts to understand the AIX os and environment and 
 8031134 : PPC64: implement printing on AIX) our port passes all but the 
 following 7 jtreg regression tests on AIX (compared to the Linux/x86_64 
 baseline from www.java.net/download/jdk8/testresults/testresults.html‎):
 
 java/net/Inet6Address/B6558853.java
 java/nio/channels/AsynchronousChannelGroup/Basic.java (sporadically)
 java/nio/channels/AsynchronousChannelGroup/GroupOfOne.java
 java/nio/channels/AsynchronousChannelGroup/Unbounded.java (sporadically)
 java/nio/channels/Selector/RacyDeregister.java
 sun/security/krb5/auto/Unreachable.java (only on IPv6)
 
 Thank you and best regards,
 Volker
 
 
 Following a detailed description of the various changes:
 src/share/native/java/util/zip/zip_util.c
 src/share/native/sun/management/DiagnosticCommandImpl.c
 
 According to ISO C it is perfectly legal for malloc to return zero if called 
 with a zero argument. Fix various places where malloc can potentially 
 correctly return zero because it was called with a zero argument.
 Also fixed DiagnosticCommandImpl.c to include stdlib.h. This only fixes a 
 compiler warning on Linux, but on AIX it prevents a VM crash later on because 
 the return value of malloc() will be casted to int which is especially bad if 
 that pointer was bigger than 32-bit.
 make/CompileJavaClasses.gmk
 
 Also use PollingWatchService on AIX.
 make/lib/NioLibraries.gmk
 src/aix/native/sun/nio/ch/AixNativeThread.c
 
 Put the implementation for the native methods of NativeThread into 
 AixNativeThread.c on AIX.
 src/solaris/native/sun/nio/ch/PollArrayWrapper.c
 src/solaris/native/sun/nio/ch/Net.c
 src/aix/classes/sun/nio/ch/AixPollPort.java
 src/aix/native/sun/nio/ch/AixPollPort.c
 src/aix/native/java/net/aix_close.c
 
 On AIX, the constants used for the polling events (i.e. POLLIN, POLLOUT, ...) 
 are defined to different values than on other operating systems. The problem 
 is however, that these constants are hardcoded as public final static members 
 of various, shared Java classes. We therefore have to map them from Java to 
 native every time before calling one of the native poll functions and back to 
 Java after the call on AIX in order to get the right semantics.
 src/share/classes/java/nio/file/CopyMoveHelper.java
 
 As discussed on the core-libs mailing list (see 
 http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-December/024119.html)
  it is not necessary to call Files.getFileAttributeView() with any 
 linkOptions because at that place we've already checked that the target file 
 can not be a symbolic link. This change makes the implementation more robust 
 on platforms which support symbolic links but do not support the O_NOFOLLOW 
 flag to the open system call. It also makes the JDK pass the 
 demo/zipfs/basic.sh test on AIX.
 src/share/classes/sun/nio/cs/ext/ExtendedCharsets.java
 
 Support compound text on AIX in the same way like on other Unix platforms.
 src/share/classes/sun/tools/attach/META-INF/services/com.sun.tools.attach.spi.AttachProvider
 
 Define the correct attach provider for AIX.
 src/solaris/native/java/net/net_util_md.h
 src/solaris/native/sun/nio/ch/FileDispatcherImpl.c
 src/solaris/native/sun/nio/ch/ServerSocketChannelImpl.c
 
 AIX needs a workaround for I/O cancellation (see: 
 http://publib.boulder.ibm.com/infocenter/pseries/v5r3/index.jsp?topic=/com.ibm.aix.basetechref/doc/basetrf1/close.htm).
  ..The close() subroutine is blocked until all subroutines which use the 
 file descriptor return to usr space. For example, when a thread is calling 
 close and another thread is calling select with the same file descriptor, the 
 close subroutine does not return until the select call returns To fix 
 this problem, we have to use the various NET_ wrappers which are declared in 
 

Re: RFR(L): 8031581: PPC64: Addons and fixes for AIX to pass the jdk regression tests

2014-01-15 Thread Alan Bateman

On 15/01/2014 06:24, David Holmes wrote:


I'm not a fan of runtime checks of this kind though if it is only a 
very samll number of values it might be okay.


Another option would be to make those classes into templates as done 
with Version.java.template and substitute the right values at build time.


But I'll let Alan and net-dev folk come back with their preferred 
technique for this.


I plan to spend time on Volker's webrev later in the week (just too busy 
with other things right now). For the translation issue then it's an 
oversight in the original implementation, it just hasn't come up before 
(to my knowledge anyway). The simplest solution here maybe to to just 
move them to sun.net.ch.Net and have them initialized to their native 
value. In general then I'm not too concerned about that one, it's the 
changes to support async close on AIX that are leaping out at me.


-Alan


Re: RFR (S): 7185591: jcmd-big-script.sh ERROR: could not find app's Java pid.

2014-01-15 Thread Yekaterina Kantserova

Staffan, thank you for pointing out performance and test.src issues!

The webrev with fixes can be found here:
http://cr.openjdk.java.net/~ykantser/7185591/webrev.01/

Please, see my comments in-lined.

Thanks,
Katja



On 01/13/2014 01:19 PM, Staffan Larsen wrote:

Katja,

test/lib/testlibrary/jdk/testlibrary/JcmdBase.java
  68  * Run jcmd standalone

I think you should expand a bit on what “standalone” means here. It took me a 
while to understand the difference.

Fixed.


test/lib/testlibrary/jdk/testlibrary/OutputAnalyzer.java
423 public int indexOf(String pattern) {

This seems very inefficient. Add all lines to an ArrayList and then walk 
through them one at a time to if it matches and then walk through them once 
again to find the index of that line.

469 public int shouldMatchByLine(String from, String to, String pattern) {

Same inefficiency here, but worse because both asLines() and indexOf() does the 
same work.

test/lib/testlibrary/jdk/testlibrary/Utils.java
65 public static final String TEST_SRC = 
System.getProperty(test.src).trim();

Fixed.


I wonder if this really works. Isn’t “test.src” different for different tests? 
A property that jtreg changes

Yes, it is different.

before invoking each test? Or does this work because each test is run in a 
different class loader and Utils.java will exist once in each class loader?
I've learned now it works because jtreg compiles all classes a test 
needs to a separate location. For example when I run TestJcmdDefaults 
there will be both TestJcmdDefaults.class and 
jdk/testlibrary/Utils.class under 
/tmp/jtreg/jtreg-workdir/classes/sun/tools/jcmd/. The class loader will 
load Utils for TestJcmdDefaults from 
/tmp/jtreg/jtreg-workdir/classes/sun/tools/jcmd/.


...
[Loaded jdk.testlibrary.Utils from 
file:/tmp/jtreg/jtreg-workdir/classes/sun/tools/jcmd/]

...

But I think it's better to declare TEST_SRC per test precisely because 
it's different for different tests.





/Staffan


On 10 jan 2014, at 13:50, Yekaterina Kantserova 
yekaterina.kantser...@oracle.com wrote:


Hi,

Could I please have a review of this fix.

In this fix I've rewritten sun/tools/jcmd/* tests in pure java to get rid of 
all intermittent failures depending on for example MKS or race conditions (test 
application has not yet started when the test start to run).


Webrev:
http://cr.openjdk.java.net/~ykantser/7185591/webrev.00/

Primal bug:
https://bugs.openjdk.java.net/browse/JDK-7185591

Similar bugs:
https://bugs.openjdk.java.net/browse/JDK-6977426
https://bugs.openjdk.java.net/browse/JDK-8020798
https://bugs.openjdk.java.net/browse/JDK-7130656 (this one is blocked by 
https://bugs.openjdk.java.net/browse/JDK-8031482 so far)

Thanks,
Katja




Re: RFR (S): 7185591: jcmd-big-script.sh ERROR: could not find app's Java pid.

2014-01-15 Thread Staffan Larsen
Katja,

test/lib/testlibrary/jdk/testlibrary/JcmdBase.java
The toolArgs parameter should be renamed jcmdArgs.

test/lib/testlibrary/jdk/testlibrary/OutputAnalyzer.java
423 public int indexOf(String pattern) {
You don’t have to convert the List to an array before iterating through it. Use 
lLines.get(i).matches(…) instead.

470 public int shouldMatchByLine(String from, String to, String pattern) {
This method still reads the lines into an ArrayList (at most) three times. It 
calls asLines() once and indexOf() twice. There could be a version of indexOf() 
that takes a ListString. Actually indexOf could be private and always take a 
ListString.


Otherwise ok!

/Staffan



On 15 jan 2014, at 10:13, Yekaterina Kantserova 
yekaterina.kantser...@oracle.com wrote:

 Staffan, thank you for pointing out performance and test.src issues!
 
 The webrev with fixes can be found here:
 http://cr.openjdk.java.net/~ykantser/7185591/webrev.01/
 
 Please, see my comments in-lined.
 
 Thanks,
 Katja
 
 
 
 On 01/13/2014 01:19 PM, Staffan Larsen wrote:
 Katja,
 
 test/lib/testlibrary/jdk/testlibrary/JcmdBase.java
  68  * Run jcmd standalone
 
 I think you should expand a bit on what “standalone” means here. It took me 
 a while to understand the difference.
 Fixed.
 
 test/lib/testlibrary/jdk/testlibrary/OutputAnalyzer.java
 423 public int indexOf(String pattern) {
 
 This seems very inefficient. Add all lines to an ArrayList and then walk 
 through them one at a time to if it matches and then walk through them once 
 again to find the index of that line.
 
 469 public int shouldMatchByLine(String from, String to, String pattern) 
 {
 
 Same inefficiency here, but worse because both asLines() and indexOf() does 
 the same work.
 
 test/lib/testlibrary/jdk/testlibrary/Utils.java
 65 public static final String TEST_SRC = 
 System.getProperty(test.src).trim();
 Fixed.
 
 I wonder if this really works. Isn’t “test.src” different for different 
 tests? A property that jtreg changes
 Yes, it is different.
 before invoking each test? Or does this work because each test is run in a 
 different class loader and Utils.java will exist once in each class loader?
 I've learned now it works because jtreg compiles all classes a test needs to 
 a separate location. For example when I run TestJcmdDefaults there will be 
 both TestJcmdDefaults.class and jdk/testlibrary/Utils.class under 
 /tmp/jtreg/jtreg-workdir/classes/sun/tools/jcmd/. The class loader will load 
 Utils for TestJcmdDefaults from 
 /tmp/jtreg/jtreg-workdir/classes/sun/tools/jcmd/.
 
 ...
 [Loaded jdk.testlibrary.Utils from 
 file:/tmp/jtreg/jtreg-workdir/classes/sun/tools/jcmd/]
 ...
 
 But I think it's better to declare TEST_SRC per test precisely because it's 
 different for different tests.
 
 
 
 /Staffan
 
 
 On 10 jan 2014, at 13:50, Yekaterina Kantserova 
 yekaterina.kantser...@oracle.com wrote:
 
 Hi,
 
 Could I please have a review of this fix.
 
 In this fix I've rewritten sun/tools/jcmd/* tests in pure java to get rid 
 of all intermittent failures depending on for example MKS or race 
 conditions (test application has not yet started when the test start to 
 run).
 
 
 Webrev:
 http://cr.openjdk.java.net/~ykantser/7185591/webrev.00/
 
 Primal bug:
 https://bugs.openjdk.java.net/browse/JDK-7185591
 
 Similar bugs:
 https://bugs.openjdk.java.net/browse/JDK-6977426
 https://bugs.openjdk.java.net/browse/JDK-8020798
 https://bugs.openjdk.java.net/browse/JDK-7130656 (this one is blocked by 
 https://bugs.openjdk.java.net/browse/JDK-8031482 so far)
 
 Thanks,
 Katja



Re: RFR(L): 8031581: PPC64: Addons and fixes for AIX to pass the jdk regression tests

2014-01-15 Thread Volker Simonis
On Wed, Jan 15, 2014 at 10:03 AM, Alan Bateman alan.bate...@oracle.comwrote:

 On 15/01/2014 06:24, David Holmes wrote:


 I'm not a fan of runtime checks of this kind though if it is only a very
 samll number of values it might be okay.

 Another option would be to make those classes into templates as done
 with Version.java.template and substitute the right values at build time.

 But I'll let Alan and net-dev folk come back with their preferred
 technique for this.

  I plan to spend time on Volker's webrev later in the week (just too busy
 with other things right now). For the translation issue then it's an
 oversight in the original implementation, it just hasn't come up before (to
 my knowledge anyway). The simplest solution here maybe to to just move them
 to sun.net.ch.Net and have them initialized to their native value.


Do you mean sun.nio.ch.Net right?

Do you propose to completely remove the definitions of the POLL constants
from:

src/share/classes/sun/nio/ch/AbstractPollArrayWrapper.java
src/solaris/classes/sun/nio/ch/Port.java

and replace all their usages by Net.POLL* ?


 In general then I'm not too concerned about that one, it's the changes to
 support async close on AIX that are leaping out at me.

 -Alan



Re: RFR (S): 7185591: jcmd-big-script.sh ERROR: could not find app's Java pid.

2014-01-15 Thread Yekaterina Kantserova

Staffan,

The webrev with changes can be found here:
http://cr.openjdk.java.net/~ykantser/7185591/webrev.02/

If it looks good, could you please be my sponsor and push the changes? 
The patch is attached to this mail.


Thanks,
Katja



On 01/15/2014 11:37 AM, Staffan Larsen wrote:

Katja,

test/lib/testlibrary/jdk/testlibrary/JcmdBase.java
The toolArgs parameter should be renamed jcmdArgs.

test/lib/testlibrary/jdk/testlibrary/OutputAnalyzer.java
423 public int indexOf(String pattern) {
You don’t have to convert the List to an array before iterating 
through it. Use lLines.get(i).matches(…) instead.


470 public int shouldMatchByLine(String from, String to, String 
pattern) {
This method still reads the lines into an ArrayList (at most) three 
times. It calls asLines() once and indexOf() twice. There could be a 
version of indexOf() that takes a ListString. Actually indexOf could 
be private and always take a ListString.



Otherwise ok!

/Staffan



On 15 jan 2014, at 10:13, Yekaterina Kantserova 
yekaterina.kantser...@oracle.com 
mailto:yekaterina.kantser...@oracle.com wrote:



Staffan, thank you for pointing out performance and test.src issues!

The webrev with fixes can be found here:
http://cr.openjdk.java.net/~ykantser/7185591/webrev.01/ 
http://cr.openjdk.java.net/%7Eykantser/7185591/webrev.01/


Please, see my comments in-lined.

Thanks,
Katja



On 01/13/2014 01:19 PM, Staffan Larsen wrote:

Katja,

test/lib/testlibrary/jdk/testlibrary/JcmdBase.java
 68  * Run jcmd standalone

I think you should expand a bit on what “standalone” means here. It 
took me a while to understand the difference.

Fixed.


test/lib/testlibrary/jdk/testlibrary/OutputAnalyzer.java
423 public int indexOf(String pattern) {

This seems very inefficient. Add all lines to an ArrayList and then 
walk through them one at a time to if it matches and then walk 
through them once again to find the index of that line.


469 public int shouldMatchByLine(String from, String to, String 
pattern) {


Same inefficiency here, but worse because both asLines() and 
indexOf() does the same work.


test/lib/testlibrary/jdk/testlibrary/Utils.java
65 public static final String TEST_SRC = 
System.getProperty(test.src).trim();

Fixed.


I wonder if this really works. Isn’t “test.src” different for 
different tests? A property that jtreg changes

Yes, it is different.
before invoking each test? Or does this work because each test is 
run in a different class loader and Utils.java will exist once in 
each class loader?
I've learned now it works because jtreg compiles all classes a test 
needs to a separate location. For example when I run TestJcmdDefaults 
there will be both TestJcmdDefaults.class and 
jdk/testlibrary/Utils.class under 
/tmp/jtreg/jtreg-workdir/classes/sun/tools/jcmd/. The class loader 
will load Utils for TestJcmdDefaults from 
/tmp/jtreg/jtreg-workdir/classes/sun/tools/jcmd/.


...
[Loaded jdk.testlibrary.Utils from 
file:/tmp/jtreg/jtreg-workdir/classes/sun/tools/jcmd/]

...

But I think it's better to declare TEST_SRC per test precisely 
because it's different for different tests.





/Staffan


On 10 jan 2014, at 13:50, Yekaterina Kantserova 
yekaterina.kantser...@oracle.com 
mailto:yekaterina.kantser...@oracle.com wrote:



Hi,

Could I please have a review of this fix.

In this fix I've rewritten sun/tools/jcmd/* tests in pure java to 
get rid of all intermittent failures depending on for example MKS 
or race conditions (test application has not yet started when the 
test start to run).



Webrev:
http://cr.openjdk.java.net/~ykantser/7185591/webrev.00/ 
http://cr.openjdk.java.net/%7Eykantser/7185591/webrev.00/


Primal bug:
https://bugs.openjdk.java.net/browse/JDK-7185591

Similar bugs:
https://bugs.openjdk.java.net/browse/JDK-6977426
https://bugs.openjdk.java.net/browse/JDK-8020798
https://bugs.openjdk.java.net/browse/JDK-7130656 (this one is 
blocked by https://bugs.openjdk.java.net/browse/JDK-8031482 so far)


Thanks,
Katja




# HG changeset patch
# User ykantser
# Date 1389791523 -3600
# Node ID 8f80cea368b700dcfba912cbc4297c4a3a8c63b0
# Parent  322a13ba1def144b685b52e7494eec45823fec31
7185591: jcmd-big-script.sh ERROR: could not find app's Java pid.
Reviewed-by: egahlin, sla, jbachorik

diff --git a/test/ProblemList.txt b/test/ProblemList.txt
--- a/test/ProblemList.txt
+++ b/test/ProblemList.txt
@@ -277,3 +277,10 @@
 # jdk_util
 
 
+
+# svc_tools
+
+# 8031482
+sun/tools/jcmd/TestJcmdSanity.javawindows-all
+
+
diff --git a/test/lib/testlibrary/jdk/testlibrary/JcmdBase.java b/test/lib/testlibrary/jdk/testlibrary/JcmdBase.java
--- a/test/lib/testlibrary/jdk/testlibrary/JcmdBase.java
+++ b/test/lib/testlibrary/jdk/testlibrary/JcmdBase.java
@@ -26,34 +26,91 @@
 import java.util.Arrays;
 
 /**
- * Super class for tests which need to attach jcmd 

Re: Review request for 7195249: Some jtreg tests use hard coded ports

2014-01-15 Thread taras ledkov

Hi,

Please take a look at the new review.

Webrev for jdk part:
http://cr.openjdk.java.net/~anazarov/7195249/jdk/webrev.02/

Webrev for hs part:
http://cr.openjdk.java.net/~anazarov/7195249/hs/webrev.02/

My answers are inline:

On 08.01.2014 17:46, Staffan Larsen wrote:

Hi Taras,

Thanks for doing this clean up and conversion of tests into Java. Here’s a 
couple of comments:

test/runtime/6294277/SourceDebugExtension.java:
This test could be simplified by not specifying an address at all. Since the test 
never connects to the JVM started with -Xrunjdwp, there is no reason to specify an 
address. If address is unspecified (and server=y), the connector will pick an 
address and print it to the command line. Thus the only change that needs to be done 
is to remove ,address=” from the @run command.

fixed


test/sun/management/jmxremote/bootstrap/RmiBootstrapTest.sh:
test/sun/management/jmxremote/bootstrap/RmiSslBootstrapTest.sh:
These tests do not compile cleanly with an empty JTwork directory. It seems 
that having one @build for each class does not work well - when compiling 
RmiBootstrapTest.java it cannot find TestLogger. Moving all classes to one 
@build statement solved this problem for me.

fixed


test/lib/testlibrary/jdk/testlibrary/ProcessTools.java:
  187 FutureVoid stdoutTask = stdout.process();
  188 FutureVoid stderrTask = stderr.process();
The stdoutTask and stderrTask variables are unused.

fixed


test/sun/management/jmxremote/bootstrap/RmiRegistrySslTest.java:
At first I thought something was wrong with this file - the diff is very weird. 
Then I realized you renamed an old file and created a new file using the old 
name.

You are right. I did it to keep the test name.


test/sun/management/jmxremote/bootstrap/AbstractFilePermissionTest.java:
- Is resetPasswordFilePermission() really necessary? It looks like you delete 
the files at the beginning of the test in any case.
I think yes. n the first place, this functionality was at the old code. 
In the second place, a file without write permission may be a problem 
for a further cleanup (not by the test, for example for the tests 
launcher scripts etc.)



- I find the names and usage of “mgmt” and “file2PermissionTest” confusing. 
They are both Paths. One is used directly by the sub-classes, the other has a 
getter method.

fixed


- Lines 57-58: Don’t swallow exceptions, add an ex.printStackTrace(). (Same 
thing for all other places where you call Integer.parseInt())

fixed


test/sun/management/jmxremote/bootstrap/Dummy.java:
This file is never used as far as I can see.
It is used by PasswordFilePermissionTest  SSLConfigFilePermissionTest 
via the AbstractFilePermissionTest (see the doTest method, 
AbstractFilePermissionTest : 162).



Thanks,
/Staffan





On 26 dec 2013, at 14:09, taras ledkov taras.led...@oracle.com wrote:


Hi,

Please take a look at the review with fixed issues about trying to launch test 
that needs free port several times.

Webrev for jdk part:
http://cr.openjdk.java.net/~anazarov/7195249/jdk/webrev.01/

Webrev for hs part:
http://cr.openjdk.java.net/~anazarov/7195249/hs/webrev.01/

Pay your attention to new method ProcessTools.startProcess(String, ProcessBuilder, 
ConsumerString) that is used to analyze all output of a sub-process.  It has 
common part with
ProcessTools.startProcess(String, ProcessBuilder, PredicateString, long, 
TumeUnit) that is used to determine the warm-up moment.

I think the ProcessTools.startProcess(String, ProcessBuilder, 
PredicateString, long, TumeUnit) may be changed by adding LinePump to stderr 
if there is not serious reason for restricting the warm-up analysis to stdout stream.

On 10.12.2013 16:16, Yekaterina Kantserova wrote:

Hi,

I've consulted with Serviceability engineers (add them to CC list) and
they would like to see tests to solve these problem so far:

2. Implement loops in every test.

Thanks,
Katja


On 12/09/2013 11:02 AM, Alexandre (Shura) Iline wrote:

Guys.

Let me try to sum up what was said before and may be suggest a
compromise.

1. There is a desire to have a support port allocation on the level of
a JTReg suite execution. Taras created a bug for that
(https://bugs.openjdk.java.net/browse/JDK-7195249). Whether it is a
test harness API or a library API does not really matter from usage
point of view.

2. There is no way to make the tests absolutely stable, whatever port
allocation logic is used. The best we could do is to try to perform
the test logic with different ports until the test succeeds.

Both arguments make sense. #2 is the ultimate answer, of course, but
better be used in conjunction with a meaningful port selection algorithm.

At the same time, copying a loop-until-success login from one test to
another may be not the best solution. Library could help with that I
believe. There only need to be an API method which takes behavior as a
parameter and run it until it succeeds. Something like:
public T runOnAFreePort(FunctionT, Integer)

Re: RFR(L): 8031581: PPC64: Addons and fixes for AIX to pass the jdk regression tests

2014-01-15 Thread Volker Simonis
Hi Staffan,

thanks for the review. Please find my comments inline:

On Wed, Jan 15, 2014 at 9:57 AM, Staffan Larsen
staffan.lar...@oracle.comwrote:

 Volker,

 I’ve look at the following files:

 src/share/native/sun/management/DiagnosticCommandImpl.c:
 nit: “legel” - “legal” (two times)
 In Java_sun_management_DiagnosticCommandImpl_getDiagnosticCommandInfo() if
 you allow dcmd_info_array to become NULL,
 then jmm_interface-GetDiagnosticCommandInfo() will throw an NPE and you
 need to check that.


Good catch. I actually had problems with malloc returning NULL in
'getDiagnosticCommandArgumentInfoArray()' and then changed all other
potentially dangerous locations which used the same pattern.

However I think if the 'dcmd_info_array' has zero length it would be
perfectly fine to return a zero length array. So what about the following
solution:

  dcmdInfoCls = (*env)-FindClass(env,
  sun/management/DiagnosticCommandInfo);
  num_commands = (*env)-GetArrayLength(env, commands);
  if (num_commands = 0) {
  result = (*env)-NewObjectArray(env, 0, dcmdInfoCls, NULL);
  if (result == NULL) {
  JNU_ThrowOutOfMemoryError(env, 0);
  }
  else {
  return result;
  }
  }
  dcmd_info_array = (dcmdInfo*) malloc(num_commands * sizeof(dcmdInfo));
  if (dcmd_info_array == NULL) {
  JNU_ThrowOutOfMemoryError(env, NULL);
  }
  jmm_interface-GetDiagnosticCommandInfo(env, commands, dcmd_info_array);
  result = (*env)-NewObjectArray(env, num_commands, dcmdInfoCls, NULL);

That seems easier and saves me from handling the exception.

What do you think?

src/solaris/native/sun/management/OperatingSystemImpl.c
 No comments.

 src/share/transport/socket/socketTransport.c
 No comments.


 src/share/classes/sun/tools/attach/META-INF/services/com.sun.tools.attach.spi.AttachProvider
 No comments.


 Thanks,
 /Staffan



 On 14 jan 2014, at 09:40, Volker Simonis volker.simo...@gmail.com wrote:

 Hi,

 could you please review the following changes for the ppc-aix-port
 stage/stage-9 repositories (the changes are planned for integration into
 ppc-aix-port/stage-9 and subsequent backporting to ppc-aix-port/stage):

 http://cr.openjdk.java.net/~simonis/webrevs/8031581/

 I've build and smoke tested without any problems on Linux/x86_64 and
 PPC64, Windows/x86_64, MacOSX, Solaris/SPARC64 and AIX7PPC64.

 With these changes (and together with the changes from 8028537: PPC64:
 Updated jdk/test scripts to understand the AIX os and environment and
 8031134 : PPC64: implement printing on AIX) our port passes all but the
 following 7 jtreg regression tests on AIX (compared to the Linux/x86_64
 baseline from www.java.net/download/jdk8/testresults/testresults.html‎):

 java/net/Inet6Address/B6558853.java
 java/nio/channels/AsynchronousChannelGroup/Basic.java (sporadically)
 java/nio/channels/AsynchronousChannelGroup/GroupOfOne.java
 java/nio/channels/AsynchronousChannelGroup/Unbounded.java (sporadically)
 java/nio/channels/Selector/RacyDeregister.java
 sun/security/krb5/auto/Unreachable.java (only on IPv6)

 Thank you and best regards,
 Volker


 Following a detailed description of the various changes:
  src/share/native/java/util/zip/zip_util.c
 src/share/native/sun/management/DiagnosticCommandImpl.c

- According to ISO C it is perfectly legal for malloc to return zero
if called with a zero argument. Fix various places where malloc can
potentially correctly return zero because it was called with a zero
argument.
- Also fixed DiagnosticCommandImpl.c to include stdlib.h. This only
fixes a compiler warning on Linux, but on AIX it prevents a VM crash later
on because the return value of malloc() will be casted to int which is
especially bad if that pointer was bigger than 32-bit.

 make/CompileJavaClasses.gmk

- Also use PollingWatchService on AIX.

 make/lib/NioLibraries.gmk
 src/aix/native/sun/nio/ch/AixNativeThread.c

- Put the implementation for the native methods of NativeThread into
AixNativeThread.c on AIX.

 src/solaris/native/sun/nio/ch/PollArrayWrapper.c
 src/solaris/native/sun/nio/ch/Net.c
 src/aix/classes/sun/nio/ch/AixPollPort.java
 src/aix/native/sun/nio/ch/AixPollPort.c
 src/aix/native/java/net/aix_close.c

- On AIX, the constants used for the polling events (i.e. POLLIN,
POLLOUT, ...) are defined to different values than on other operating
systems. The problem is however, that these constants are hardcoded as
public final static members of various, shared Java classes. We therefore
have to map them from Java to native every time before calling one of the
native poll functions and back to Java after the call on AIX in order to
get the right semantics.

 src/share/classes/java/nio/file/CopyMoveHelper.java

- As discussed on the core-libs mailing list (see

 http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-December/024119.html)
it is not necessary to call 

hg: jdk8/tl/jdk: 8031502: JSR292: IncompatibleClassChangeError in LambdaForm for CharSequence.toString() method handle type converter

2014-01-15 Thread eric . mccorkle
Changeset: 9e91793fd516
Author:vlivanov
Date:  2014-01-15 20:48 +0400
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/9e91793fd516

8031502: JSR292: IncompatibleClassChangeError in LambdaForm for 
CharSequence.toString() method handle type converter
Reviewed-by: sundar, lagergren, drchase

! src/share/classes/java/lang/invoke/InvokerBytecodeGenerator.java
+ test/java/lang/invoke/ObjectMethodInInterfaceTest.java



Re: RFR(L): 8031581: PPC64: Addons and fixes for AIX to pass the jdk regression tests

2014-01-15 Thread Volker Simonis
On Wed, Jan 15, 2014 at 6:27 PM, Volker Simonis volker.simo...@gmail.com
wrote:
 On Wed, Jan 15, 2014 at 5:34 PM, Volker Simonis
 volker.simo...@gmail.com wrote:
 Hi Staffan,

 thanks for the review. Please find my comments inline:

 On Wed, Jan 15, 2014 at 9:57 AM, Staffan Larsen 
staffan.lar...@oracle.com
 wrote:

 Volker,

 I’ve look at the following files:

 src/share/native/sun/management/DiagnosticCommandImpl.c:
 nit: “legel” - “legal” (two times)
 In Java_sun_management_DiagnosticCommandImpl_getDiagnosticCommandInfo()
if
 you allow dcmd_info_array to become NULL, then
 jmm_interface-GetDiagnosticCommandInfo() will throw an NPE and you
need to
 check that.


 Good catch. I actually had problems with malloc returning NULL in
 'getDiagnosticCommandArgumentInfoArray()' and then changed all other
 potentially dangerous locations which used the same pattern.

 However I think if the 'dcmd_info_array' has zero length it would be
 perfectly fine to return a zero length array. So what about the following
 solution:


Sorry for the noise - it seems I was a little indisposed during the last
mails:)
So this is the simple change I'd like to propose for
Java_sun_management_DiagnosticCommandImpl_getDiagnosticCommandInfo():


@@ -117,19 +119,23 @@
   return NULL;
   }
   num_commands = (*env)-GetArrayLength(env, commands);
-  dcmd_info_array = (dcmdInfo*) malloc(num_commands *
-   sizeof(dcmdInfo));
+  dcmdInfoCls = (*env)-FindClass(env,
+  sun/management/DiagnosticCommandInfo);
+  result = (*env)-NewObjectArray(env, num_commands, dcmdInfoCls, NULL);
+  if (result == NULL) {
+  JNU_ThrowOutOfMemoryError(env, 0);
+  }
+  if (num_commands == 0) {
+  /* Handle the 'zero commands' case specially to avoid calling
'malloc()' */
+  /* with a zero argument because that may legally return a NULL
pointer.  */
+  return result;
+  }
+  dcmd_info_array = (dcmdInfo*) malloc(num_commands * sizeof(dcmdInfo));
   if (dcmd_info_array == NULL) {
   JNU_ThrowOutOfMemoryError(env, NULL);
   }
   jmm_interface-GetDiagnosticCommandInfo(env, commands, dcmd_info_array);
-  dcmdInfoCls = (*env)-FindClass(env,
-  sun/management/DiagnosticCommandInfo);
-  result = (*env)-NewObjectArray(env, num_commands, dcmdInfoCls, NULL);
-  if (result == NULL) {
-  free(dcmd_info_array);
-  JNU_ThrowOutOfMemoryError(env, 0);
-  }
   for (i=0; inum_commands; i++) {
   args = getDiagnosticCommandArgumentInfoArray(env,

(*env)-GetObjectArrayElement(env,commands,i),

If the 'commands' input array is of zero length just return a zero length
array.
OK?

   dcmdInfoCls = (*env)-FindClass(env,

sun/management/DiagnosticCommandInfo);
   num_commands = (*env)-GetArrayLength(env, commands);

 Sorry, of course I wanted to say if (num_commands == 0) here!

   if (num_commands = 0) {
   result = (*env)-NewObjectArray(env, 0, dcmdInfoCls, NULL);
   if (result == NULL) {
   JNU_ThrowOutOfMemoryError(env, 0);
   }
   else {
   return result;
   }
   }
   dcmd_info_array = (dcmdInfo*) malloc(num_commands * sizeof(dcmdInfo));
   if (dcmd_info_array == NULL) {
   JNU_ThrowOutOfMemoryError(env, NULL);
   }
   jmm_interface-GetDiagnosticCommandInfo(env, commands,
dcmd_info_array);
   result = (*env)-NewObjectArray(env, num_commands, dcmdInfoCls, NULL);

 That seems easier and saves me from handling the exception.

 What do you think?

 src/solaris/native/sun/management/OperatingSystemImpl.c
 No comments.

 src/share/transport/socket/socketTransport.c
 No comments.



src/share/classes/sun/tools/attach/META-INF/services/com.sun.tools.attach.spi.AttachProvider
 No comments.


 Thanks,
 /Staffan



 On 14 jan 2014, at 09:40, Volker Simonis volker.simo...@gmail.com
wrote:

 Hi,

 could you please review the following changes for the ppc-aix-port
 stage/stage-9 repositories (the changes are planned for integration into
 ppc-aix-port/stage-9 and subsequent backporting to ppc-aix-port/stage):

 http://cr.openjdk.java.net/~simonis/webrevs/8031581/

 I've build and smoke tested without any problems on Linux/x86_64 and
 PPC64, Windows/x86_64, MacOSX, Solaris/SPARC64 and AIX7PPC64.

 With these changes (and together with the changes from 8028537: PPC64:
 Updated jdk/test scripts to understand the AIX os and environment and
 8031134 : PPC64: implement printing on AIX) our port passes all but
the
 following 7 jtreg regression tests on AIX (compared to the Linux/x86_64
 baseline from www.java.net/download/jdk8/testresults/testresults.html‎):

 java/net/Inet6Address/B6558853.java
 java/nio/channels/AsynchronousChannelGroup/Basic.java (sporadically)
 java/nio/channels/AsynchronousChannelGroup/GroupOfOne.java
 java/nio/channels/AsynchronousChannelGroup/Unbounded.java (sporadically)
 java/nio/channels/Selector/RacyDeregister.java
 sun/security/krb5/auto/Unreachable.java (only on IPv6)

 Thank you and best regards,
 

RR(XS): JDK-8024049 com/sun/jdi/ProcessAttachTest.sh shortens 7-digit pid to 6-digit

2014-01-15 Thread Dmitry Samersoff
Hi Everybody,

Please review small, test-only fix.

http://cr.openjdk.java.net/~dsamersoff/JDK-8024049/webrev.01/

fix summary:

  grep/cut chain is replaced with awk.

-- 
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* I would love to change the world, but they won't give me the sources.


Re: RFR(L): 8031581: PPC64: Addons and fixes for AIX to pass the jdk regression tests

2014-01-15 Thread Staffan Larsen

On 15 jan 2014, at 18:27, Volker Simonis volker.simo...@gmail.com wrote:

 On Wed, Jan 15, 2014 at 5:34 PM, Volker Simonis
 volker.simo...@gmail.com wrote:
 Hi Staffan,
 
 thanks for the review. Please find my comments inline:
 
 On Wed, Jan 15, 2014 at 9:57 AM, Staffan Larsen staffan.lar...@oracle.com
 wrote:
 
 Volker,
 
 I’ve look at the following files:
 
 src/share/native/sun/management/DiagnosticCommandImpl.c:
 nit: “legel” - “legal” (two times)
 In Java_sun_management_DiagnosticCommandImpl_getDiagnosticCommandInfo() if
 you allow dcmd_info_array to become NULL, then
 jmm_interface-GetDiagnosticCommandInfo() will throw an NPE and you need to
 check that.
 
 
 Good catch. I actually had problems with malloc returning NULL in
 'getDiagnosticCommandArgumentInfoArray()' and then changed all other
 potentially dangerous locations which used the same pattern.
 
 However I think if the 'dcmd_info_array' has zero length it would be
 perfectly fine to return a zero length array. So what about the following
 solution:
 
  dcmdInfoCls = (*env)-FindClass(env,
  sun/management/DiagnosticCommandInfo);
  num_commands = (*env)-GetArrayLength(env, commands);
 
 Sorry, of course I wanted to say if (num_commands == 0) here!

I understood as much :-)

 
  if (num_commands = 0) {
  result = (*env)-NewObjectArray(env, 0, dcmdInfoCls, NULL);
  if (result == NULL) {
  JNU_ThrowOutOfMemoryError(env, 0);
  }
  else {
  return result;
  }
  }
  dcmd_info_array = (dcmdInfo*) malloc(num_commands * sizeof(dcmdInfo));
  if (dcmd_info_array == NULL) {
  JNU_ThrowOutOfMemoryError(env, NULL);
  }
  jmm_interface-GetDiagnosticCommandInfo(env, commands, dcmd_info_array);
  result = (*env)-NewObjectArray(env, num_commands, dcmdInfoCls, NULL);
 
 That seems easier and saves me from handling the exception.
 
 What do you think?
 
 src/solaris/native/sun/management/OperatingSystemImpl.c
 No comments.
 
 src/share/transport/socket/socketTransport.c
 No comments.
 
 
 src/share/classes/sun/tools/attach/META-INF/services/com.sun.tools.attach.spi.AttachProvider
 No comments.
 
 
 Thanks,
 /Staffan
 
 
 
 On 14 jan 2014, at 09:40, Volker Simonis volker.simo...@gmail.com wrote:
 
 Hi,
 
 could you please review the following changes for the ppc-aix-port
 stage/stage-9 repositories (the changes are planned for integration into
 ppc-aix-port/stage-9 and subsequent backporting to ppc-aix-port/stage):
 
 http://cr.openjdk.java.net/~simonis/webrevs/8031581/
 
 I've build and smoke tested without any problems on Linux/x86_64 and
 PPC64, Windows/x86_64, MacOSX, Solaris/SPARC64 and AIX7PPC64.
 
 With these changes (and together with the changes from 8028537: PPC64:
 Updated jdk/test scripts to understand the AIX os and environment and
 8031134 : PPC64: implement printing on AIX) our port passes all but the
 following 7 jtreg regression tests on AIX (compared to the Linux/x86_64
 baseline from www.java.net/download/jdk8/testresults/testresults.html‎):
 
 java/net/Inet6Address/B6558853.java
 java/nio/channels/AsynchronousChannelGroup/Basic.java (sporadically)
 java/nio/channels/AsynchronousChannelGroup/GroupOfOne.java
 java/nio/channels/AsynchronousChannelGroup/Unbounded.java (sporadically)
 java/nio/channels/Selector/RacyDeregister.java
 sun/security/krb5/auto/Unreachable.java (only on IPv6)
 
 Thank you and best regards,
 Volker
 
 
 Following a detailed description of the various changes:
 
 src/share/native/java/util/zip/zip_util.c
 src/share/native/sun/management/DiagnosticCommandImpl.c
 
 According to ISO C it is perfectly legal for malloc to return zero if
 called with a zero argument. Fix various places where malloc can potentially
 correctly return zero because it was called with a zero argument.
 Also fixed DiagnosticCommandImpl.c to include stdlib.h. This only fixes a
 compiler warning on Linux, but on AIX it prevents a VM crash later on
 because the return value of malloc() will be casted to int which is
 especially bad if that pointer was bigger than 32-bit.
 
 make/CompileJavaClasses.gmk
 
 Also use PollingWatchService on AIX.
 
 make/lib/NioLibraries.gmk
 src/aix/native/sun/nio/ch/AixNativeThread.c
 
 Put the implementation for the native methods of NativeThread into
 AixNativeThread.c on AIX.
 
 src/solaris/native/sun/nio/ch/PollArrayWrapper.c
 src/solaris/native/sun/nio/ch/Net.c
 src/aix/classes/sun/nio/ch/AixPollPort.java
 src/aix/native/sun/nio/ch/AixPollPort.c
 src/aix/native/java/net/aix_close.c
 
 On AIX, the constants used for the polling events (i.e. POLLIN, POLLOUT,
 ...) are defined to different values than on other operating systems. The
 problem is however, that these constants are hardcoded as public final
 static members of various, shared Java classes. We therefore have to map
 them from Java to native every time before calling one of the native poll
 functions and back to Java after the call on AIX in order to get the right
 semantics.
 
 

Re: RFR(L): 8031581: PPC64: Addons and fixes for AIX to pass the jdk regression tests

2014-01-15 Thread Staffan Larsen
Yes, that looks like a good solution.

/Staffan

On 15 jan 2014, at 17:34, Volker Simonis volker.simo...@gmail.com wrote:

 Hi Staffan,
 
 thanks for the review. Please find my comments inline:
 
 On Wed, Jan 15, 2014 at 9:57 AM, Staffan Larsen staffan.lar...@oracle.com 
 wrote:
 Volker,
 
 I’ve look at the following files:
 
 src/share/native/sun/management/DiagnosticCommandImpl.c:
 nit: “legel” - “legal” (two times)
 In Java_sun_management_DiagnosticCommandImpl_getDiagnosticCommandInfo() if 
 you allow dcmd_info_array to become NULL, then 
 jmm_interface-GetDiagnosticCommandInfo() will throw an NPE and you need to 
 check that.
 
 Good catch. I actually had problems with malloc returning NULL in 
 'getDiagnosticCommandArgumentInfoArray()' and then changed all other 
 potentially dangerous locations which used the same pattern.
 
 However I think if the 'dcmd_info_array' has zero length it would be 
 perfectly fine to return a zero length array. So what about the following 
 solution:
 
   dcmdInfoCls = (*env)-FindClass(env,
   sun/management/DiagnosticCommandInfo);
   num_commands = (*env)-GetArrayLength(env, commands);
   if (num_commands = 0) {
   result = (*env)-NewObjectArray(env, 0, dcmdInfoCls, NULL);
   if (result == NULL) {
   JNU_ThrowOutOfMemoryError(env, 0);
   }
   else {
   return result;
   }
   }
   dcmd_info_array = (dcmdInfo*) malloc(num_commands * sizeof(dcmdInfo));
   if (dcmd_info_array == NULL) {
   JNU_ThrowOutOfMemoryError(env, NULL);
   }
   jmm_interface-GetDiagnosticCommandInfo(env, commands, dcmd_info_array);
   result = (*env)-NewObjectArray(env, num_commands, dcmdInfoCls, NULL);
 
 That seems easier and saves me from handling the exception.
 
 What do you think?
 
 src/solaris/native/sun/management/OperatingSystemImpl.c
 No comments.
 
 src/share/transport/socket/socketTransport.c
 No comments.
 
 src/share/classes/sun/tools/attach/META-INF/services/com.sun.tools.attach.spi.AttachProvider
 No comments.
 
 
 Thanks,
 /Staffan
 
 
 
 On 14 jan 2014, at 09:40, Volker Simonis volker.simo...@gmail.com wrote:
 
 Hi,
 
 could you please review the following changes for the ppc-aix-port 
 stage/stage-9 repositories (the changes are planned for integration into 
 ppc-aix-port/stage-9 and subsequent backporting to ppc-aix-port/stage):
 
 http://cr.openjdk.java.net/~simonis/webrevs/8031581/
 
 I've build and smoke tested without any problems on Linux/x86_64 and PPC64, 
 Windows/x86_64, MacOSX, Solaris/SPARC64 and AIX7PPC64.
 
 With these changes (and together with the changes from 8028537: PPC64: 
 Updated jdk/test scripts to understand the AIX os and environment and 
 8031134 : PPC64: implement printing on AIX) our port passes all but the 
 following 7 jtreg regression tests on AIX (compared to the Linux/x86_64 
 baseline from www.java.net/download/jdk8/testresults/testresults.html‎):
 
 java/net/Inet6Address/B6558853.java
 java/nio/channels/AsynchronousChannelGroup/Basic.java (sporadically)
 java/nio/channels/AsynchronousChannelGroup/GroupOfOne.java
 java/nio/channels/AsynchronousChannelGroup/Unbounded.java (sporadically)
 java/nio/channels/Selector/RacyDeregister.java
 sun/security/krb5/auto/Unreachable.java (only on IPv6)
 
 Thank you and best regards,
 Volker
 
 
 Following a detailed description of the various changes:
 src/share/native/java/util/zip/zip_util.c
 src/share/native/sun/management/DiagnosticCommandImpl.c
 
 According to ISO C it is perfectly legal for malloc to return zero if called 
 with a zero argument. Fix various places where malloc can potentially 
 correctly return zero because it was called with a zero argument.
 Also fixed DiagnosticCommandImpl.c to include stdlib.h. This only fixes a 
 compiler warning on Linux, but on AIX it prevents a VM crash later on 
 because the return value of malloc() will be casted to int which is 
 especially bad if that pointer was bigger than 32-bit.
 make/CompileJavaClasses.gmk
 
 Also use PollingWatchService on AIX.
 make/lib/NioLibraries.gmk
 src/aix/native/sun/nio/ch/AixNativeThread.c
 
 Put the implementation for the native methods of NativeThread into 
 AixNativeThread.c on AIX.
 src/solaris/native/sun/nio/ch/PollArrayWrapper.c
 src/solaris/native/sun/nio/ch/Net.c
 src/aix/classes/sun/nio/ch/AixPollPort.java
 src/aix/native/sun/nio/ch/AixPollPort.c
 src/aix/native/java/net/aix_close.c
 
 On AIX, the constants used for the polling events (i.e. POLLIN, POLLOUT, 
 ...) are defined to different values than on other operating systems. The 
 problem is however, that these constants are hardcoded as public final 
 static members of various, shared Java classes. We therefore have to map 
 them from Java to native every time before calling one of the native poll 
 functions and back to Java after the call on AIX in order to get the right 
 semantics.
 src/share/classes/java/nio/file/CopyMoveHelper.java
 
 As discussed on the core-libs mailing list (see 
 

Re: RFR(L): 8031581: PPC64: Addons and fixes for AIX to pass the jdk regression tests

2014-01-15 Thread Staffan Larsen

On 15 jan 2014, at 18:52, Volker Simonis volker.simo...@gmail.com wrote:

 
 
 On Wed, Jan 15, 2014 at 6:27 PM, Volker Simonis volker.simo...@gmail.com 
 wrote:
  On Wed, Jan 15, 2014 at 5:34 PM, Volker Simonis
  volker.simo...@gmail.com wrote:
  Hi Staffan,
 
  thanks for the review. Please find my comments inline:
 
  On Wed, Jan 15, 2014 at 9:57 AM, Staffan Larsen staffan.lar...@oracle.com
  wrote:
 
  Volker,
 
  I’ve look at the following files:
 
  src/share/native/sun/management/DiagnosticCommandImpl.c:
  nit: “legel” - “legal” (two times)
  In Java_sun_management_DiagnosticCommandImpl_getDiagnosticCommandInfo() if
  you allow dcmd_info_array to become NULL, then
  jmm_interface-GetDiagnosticCommandInfo() will throw an NPE and you need 
  to
  check that.
 
 
  Good catch. I actually had problems with malloc returning NULL in
  'getDiagnosticCommandArgumentInfoArray()' and then changed all other
  potentially dangerous locations which used the same pattern.
 
  However I think if the 'dcmd_info_array' has zero length it would be
  perfectly fine to return a zero length array. So what about the following
  solution:
 
 
 Sorry for the noise - it seems I was a little indisposed during the last 
 mails:)
 So this is the simple change I'd like to propose for 
 Java_sun_management_DiagnosticCommandImpl_getDiagnosticCommandInfo():
 
 
 @@ -117,19 +119,23 @@
return NULL;
}
num_commands = (*env)-GetArrayLength(env, commands);
 -  dcmd_info_array = (dcmdInfo*) malloc(num_commands *
 -   sizeof(dcmdInfo));
 +  dcmdInfoCls = (*env)-FindClass(env,
 +  sun/management/DiagnosticCommandInfo);
 +  result = (*env)-NewObjectArray(env, num_commands, dcmdInfoCls, NULL);
 +  if (result == NULL) {
 +  JNU_ThrowOutOfMemoryError(env, 0);
 +  }
 +  if (num_commands == 0) {
 +  /* Handle the 'zero commands' case specially to avoid calling 
 'malloc()' */
 +  /* with a zero argument because that may legally return a NULL 
 pointer.  */
 +  return result;
 +  }
 +  dcmd_info_array = (dcmdInfo*) malloc(num_commands * sizeof(dcmdInfo));
if (dcmd_info_array == NULL) {
JNU_ThrowOutOfMemoryError(env, NULL);
}
jmm_interface-GetDiagnosticCommandInfo(env, commands, dcmd_info_array);
 -  dcmdInfoCls = (*env)-FindClass(env,
 -  sun/management/DiagnosticCommandInfo);
 -  result = (*env)-NewObjectArray(env, num_commands, dcmdInfoCls, NULL);
 -  if (result == NULL) {
 -  free(dcmd_info_array);
 -  JNU_ThrowOutOfMemoryError(env, 0);
 -  }
for (i=0; inum_commands; i++) {
args = getDiagnosticCommandArgumentInfoArray(env,
 
 (*env)-GetObjectArrayElement(env,commands,i),
 
 If the 'commands' input array is of zero length just return a zero length 
 array.
 OK?

Yes, this looks good (still :-) )

/Staffan


 
dcmdInfoCls = (*env)-FindClass(env,
sun/management/DiagnosticCommandInfo);
num_commands = (*env)-GetArrayLength(env, commands);
 
  Sorry, of course I wanted to say if (num_commands == 0) here!
 
if (num_commands = 0) {
result = (*env)-NewObjectArray(env, 0, dcmdInfoCls, NULL);
if (result == NULL) {
JNU_ThrowOutOfMemoryError(env, 0);
}
else {
return result;
}
}
dcmd_info_array = (dcmdInfo*) malloc(num_commands * sizeof(dcmdInfo));
if (dcmd_info_array == NULL) {
JNU_ThrowOutOfMemoryError(env, NULL);
}
jmm_interface-GetDiagnosticCommandInfo(env, commands, dcmd_info_array);
result = (*env)-NewObjectArray(env, num_commands, dcmdInfoCls, NULL);
 
  That seems easier and saves me from handling the exception.
 
  What do you think?
 
  src/solaris/native/sun/management/OperatingSystemImpl.c
  No comments.
 
  src/share/transport/socket/socketTransport.c
  No comments.
 
 
  src/share/classes/sun/tools/attach/META-INF/services/com.sun.tools.attach.spi.AttachProvider
  No comments.
 
 
  Thanks,
  /Staffan
 
 
 
  On 14 jan 2014, at 09:40, Volker Simonis volker.simo...@gmail.com wrote:
 
  Hi,
 
  could you please review the following changes for the ppc-aix-port
  stage/stage-9 repositories (the changes are planned for integration into
  ppc-aix-port/stage-9 and subsequent backporting to ppc-aix-port/stage):
 
  http://cr.openjdk.java.net/~simonis/webrevs/8031581/
 
  I've build and smoke tested without any problems on Linux/x86_64 and
  PPC64, Windows/x86_64, MacOSX, Solaris/SPARC64 and AIX7PPC64.
 
  With these changes (and together with the changes from 8028537: PPC64:
  Updated jdk/test scripts to understand the AIX os and environment and
  8031134 : PPC64: implement printing on AIX) our port passes all but the
  following 7 jtreg regression tests on AIX (compared to the Linux/x86_64
  baseline from www.java.net/download/jdk8/testresults/testresults.html‎):
 
  

Re: RR(XS): JDK-8024049 com/sun/jdi/ProcessAttachTest.sh shortens 7-digit pid to 6-digit

2014-01-15 Thread Staffan Larsen
Looks good!

Thanks,
/Staffan

On 15 jan 2014, at 19:23, Dmitry Samersoff dmitry.samers...@oracle.com wrote:

 Hi Everybody,
 
 Please review small, test-only fix.
 
 http://cr.openjdk.java.net/~dsamersoff/JDK-8024049/webrev.01/
 
 fix summary:
 
  grep/cut chain is replaced with awk.
 
 -- 
 Dmitry Samersoff
 Oracle Java development team, Saint Petersburg, Russia
 * I would love to change the world, but they won't give me the sources.



Review request for 8029378: com/sun/jdi/BadHandshakeTest.java failed with java.util.concurrent.TimeoutException

2014-01-15 Thread shanliang

Hi,

Please review this simple fix, the test needs  more time to wait 
Phaser.awaitAdvanceInterruptibly(...).


webrev: http://cr.openjdk.java.net/~sjiang/JDK-8029378/00/
bug: https://bugs.openjdk.java.net/browse/JDK-8029378

Thanks,
Shanliang