Please review fix for 7146424: Wildcard expansion for single entry classpath

[Kumar Srinivasan]

Hi David,
> Hi Kumar,
>
> Don't meant to be difficult but a global int i shared across the two 
> loops is not what I was suggesting rather that the loop variable be 
> declared in the loop eg:
>
>  114     for (int i = 0 ; i < margc ; i++) {
>  115         margv[i] = stdargs[i].arg;
>  116     }
>
> then you can change 117 to not use i
>
>  117     margv[i] = NULL;
>
> becomes
>
>          margv[margc] = NULL;

Ah!, oh sorry that won't fly this is c code :-( , not c++, MSC does  not 
allow it.

Kumar


>
> David
>
> On 31/07/2012 6:11 AM, Kumar Srinivasan wrote:
>> Hi David,
>>
>> Here are the changes you suggested, removed 2 scopes in
>> main.c
>>
>> The delta from last reviewed revision:
>> http://cr.openjdk.java.net/~ksrini/7146424/webrev.1/webrev.delta/index.html 
>>
>>
>> The full webrev:
>> http://cr.openjdk.java.net/~ksrini/7146424/webrev.1/index.html
>>
>> Thanks
>> Kumar
>>
>>> Hi Kumar,
>>>
>>> I'm always uncomfortable when I see common code that is effectively a
>>> no-op on all but one platform - as it means it isn't really common, we
>>> just factored it that way. But I'm not vehemently opposed. :)
>>>
>>> David
>>>
>>> On 30/07/2012 10:32 PM, Kumar Srinivasan wrote:
>>>> HI David,
>>>>
>>>>> I can't comment on the details of the actual expansion logic, nor the
>>>>> tests.
>>>>
>>>> Akhil and I have gone over this.
>>>>
>>>>>
>>>>> Looking at the overall structure I'm still unclear why more of this
>>>>> isn't just hidden in win32 only files. Why do the new JLI_* methods
>>>>> have to be JLI methods? I would have hoped that everything could be
>>>>> hidden/handled inside CreateApplicationArgs/
>>>>
>>>> We need JLI_* methods because all of the launcher's implementation 
>>>> is in
>>>> the library libjli.so on Unix
>>>> and on windows, jli.dll.
>>>>
>>>> Now, main.c is a common stub which provides the main/Winmain for 
>>>> all the
>>>> launchers, meaning java
>>>> as well as javac, javap etc. therefore main.c is linked with libjli.so
>>>> and all of them call into the JLI_Launch an
>>>> entry point which starts the launch, with the argc, argv, Note that
>>>> substantial argument processing is performed much before we reach
>>>> CreateApplicationArgs
>>>>
>>>> Since this expansion is required before we call into JLI_Launch, we 
>>>> need
>>>> to call JLI_CmdToArgs
>>>> specifically for Windows, for our parsed arguments, so that we can
>>>> substitute argc, argv with our parsed version.
>>>>
>>>> Does that answer your question ? So do you have reservations about
>>>> exposing the JLI_* interfaces,
>>>> I think it is possible to encapsulate these completely within the jli
>>>> library, thus not needing to
>>>> export those interfaces, but that will complicate the logic within
>>>> JLI_Launch, requiring platform
>>>> specific expansions functions. I think what we have now is a lot
>>>> cleaner.
>>>>
>>>>>
>>>>> One specific comment:
>>>>>
>>>>> share/bin/main.c:
>>>>>
>>>>> 99 #ifdef _WIN32
>>>>> 100 {
>>>>> 101 int i = 0;
>>>>> 102 if (getenv(JLDEBUG_ENV_ENTRY) != NULL) {
>>>>> 103 printf("Windows original main args:\n");
>>>>> 104 for (i = 0 ; i < __argc ; i++) {
>>>>> 105 printf("wwwd_args[%d] = %s\n", i, __argv[i]);
>>>>> 106 }
>>>>> 107 }
>>>>> 108 }
>>>>>
>>>>> Does MSC not permit declaration of i inside the for loop? It avoids
>>>>> the need for the extra scope.
>>>> MSC permits, there are two uses of the for-loop counters, I guess I 
>>>> can
>>>> create one variable to handle
>>>> both, and eliminate the scopes.
>>>>
>>>> Kumar
>>>>
>>>>>
>>>>> David
>>>>> -----
>>>>>
>>>>> On 27/07/2012 10:41 PM, Kumar Srinivasan wrote:
>>>>>> Hi,
>>>>>>
>>>>>> Please review the fix
>>>>>> http://cr.openjdk.java.net/~ksrini/7146424/webrev.0/
>>>>>>
>>>>>> to address:
>>>>>> 7146424: Wildcard expansion for single entry classpath
>>>>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7146424
>>>>>> and
>>>>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7167744
>>>>>>
>>>>>>
>>>>>> Notes:
>>>>>> a. cmdtoargs.c will be pushed as a separate changeset using a
>>>>>> separate CR
>>>>>> and with contributor attribution to akhil.arora at oracle.com
>>>>>>
>>>>>> b. src/solaris/bin/java_md.c is a redundant file and will be 
>>>>>> removed,
>>>>>> webrev for whatever reason is not reporting it.
>>>>>>
>>>>>> Thanks
>>>>>>
>>>>>> Kumar
>>>>>>
>>>>>>
>>>>>>
>>>>
>>