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

[Kumar Srinivasan]

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
>>>>
>>>>
>>>>
>>