Neil,

Some nits:

TestSpecialArgs.java:

extra space
105 for ( String line : tr.testOutput) {



This is very C'ish, I suggest.
 -106                 int index;
 -107                 if ((index = line.indexOf(envVarPidString)) >= 0) {

 +106                 int index =  line.indexOf(envVarPidString);
 +107                 if (index >= 0) {


Needs space
-146 launcherPid = line.substring(sindex,tindex);
+146 launcherPid = line.substring(sindex, tindex);

Needs space

-168             if (!tr.contains("NativeMemoryTracking: got value 
"+NMT_Option_Value)) {
+168             if (!tr.contains("NativeMemoryTracking: got value " + 
NMT_Option_Value)) {


Thanks
Kumar


On 6/23/2014 10:26 AM, Neil Toda wrote:

I've spun a new webrev based on the comments for webrev-03:

    http://cr.openjdk.java.net/~ntoda/8042469/webrev-04/

Changes are:

  1) java.c
        a) add free(envName) line 1063
        b) change from malloc() to JLI_MemAlloc() @ lines 1048 and 1056

Thanks

-neil

On 6/20/2014 4:45 PM, Kumar Srinivasan wrote:
Neil,

Generally looks good, yes JLI_* functions must be used, I missed that one.
Are you going to post another iteration ?

Kumar

On 6/20/2014 4:27 PM, Neil Toda wrote:

Thanks Joe.  It would have checked for NULL for me.
I'll use the JLI wrapper.

-neil

On 6/20/2014 4:04 PM, Joe Darcy wrote:
Memory allocation in the launcher should use one of the JLI_MemAlloc wrappers, if possible.

-Joe


On 06/20/2014 09:50 AM, Neil Toda wrote:

They should complain. Thanks Zhengyu. I'll make sure these are non-null.

-neil

On 6/20/2014 5:01 AM, Zhengyu Gu wrote:
Neil,

Thanks for quick implementation.

java.c:
Did not check return values of malloc(), I wonder if source code analyzers will complain.

-Zhengyu

On 6/19/2014 8:29 PM, Neil Toda wrote:

Launcher support for modified Native Memory Tracking mechanism in JVM in JDK9.

Webrev  : http://cr.openjdk.java.net/~ntoda/8042469/webrev-03/
bug         : https://bugs.openjdk.java.net/browse/JDK-8042469
CCC         : http://ccc.us.oracle.com/8042469

Thanks.

-neil








Reply via email to