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.ar...@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




Reply via email to