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

[David Holmes]

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