On Jul 30 2012, at 14:43 , David Holmes wrote:
> On 31/07/2012 7:32 AM, Kumar Srinivasan wrote:
>> 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.
>
> Wow that is unbelievable! for-loop variable declarations are part of C99 :(
>
> David
Sadly Microsoft does not and will will not be providing a C99 compiler.
http://herbsutter.com/2012/05/03/reader-qa-what-about-vc-and-c99/
Mike
> -----
>
>> 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.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
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>
>>>>
>>
