Review request 8136930: Simplify use of module-system options by custom launchers

[Kumar Srinivasan]

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 <kumar.x.srinivasan at oracle.com> 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