Mandy,

Looks good to me.

Thanks
Kumar

On 8/8/2016 3:49 PM, Mandy Chung wrote:
Kumar,

Thanks for the review.

On Aug 8, 2016, at 2:46 PM, Kumar Srinivasan <[email protected]> 
wrote:


I looked at the launcher specified changes, some minor
nits...

jdk/src/java.base/share/native/libjli/java.c

+    def_len = JLI_StrLen(option)+1+JLI_StrLen(arg)+1;
spaces after operators.

size_t buflen = JLI_StrLen(option)+2+JLI_StrLen(value);
spaces after operators.

OK.

+    arg = *(argv+1);
spaces after operators.

I see this convention (no space) being used in this file when it’s used as 
array index or pointer increment.

It reads fine without space afer operator.  Are you okay to keep it as is?

+        if (IsLauncherMainOption(arg)) {
+            kind = LAUNCHER_MAIN_OPTION;
+        } else {
+            kind = LAUNCHER_OPTION_WITH_ARGUMENT;
+        }

Could be turned into a unary operation.

Sure.

I am curious about LauncherHelper.java, there are two new imports
but the code added should not need those imports, were/are they unused
imports ?

Good catch.  I took them out.  I also

   63 import java.security.AccessController;
   64 import java.security.PrivilegedAction;

Updated webrev:
   
http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8136930/gnu-options/webrev.01/

Mandy

Reply via email to