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