Re: [v2 PATCH] ucrtbase: Implement _get_narrow_winmain_command_line/_get_wide_winmain_command_line
Nikolay Sivov <nsivov(a)codeweavers.com> wrote:
+char* CDECL _get_narrow_winmain_command_line(void) +{ + static char *narrow_command_line; + char *s; + + if (narrow_command_line) + return narrow_command_line; + + s = GetCommandLineA(); + if (*s == '"') + { + /* skip all leading quotes */ + while (*++s == '"') + ; + /* skip everything up to next quote */ + while (*s && *s++ != '"') + ; + } + else + { + while (*s && *s != ' ' && *s != '\t') + { + /* allow embedded quoted blocks */ + if (*s == '"') + { + s++; + while (*s && *s++ != '"') + ; + } + else + s++; + } + } + while (*s == ' ' || *s == '\t') + s++; + + return narrow_command_line = s; +}
Once again: Where did you get an idea of such an implementation? Is this copied from some place or based on some other implementation? Why for instance simple 'return GetCommandLineA()' is not enough as an initial implementation? -- Dmitry.
On 07/26/16 04:32, Dmitry Timoshkov wrote:
Once again: Where did you get an idea of such an implementation? Is this copied from some place or based on some other implementation? Why for instance simple 'return GetCommandLineA()' is not enough as an initial implementation?
Probably I'm just stating obvious but the function skips executable name from the command line. That's why returning what GetCommandLineA returns is incorrect. Thanks, Piotr
Piotr Caban <piotr.caban(a)gmail.com> wrote:
Once again: Where did you get an idea of such an implementation? Is this copied from some place or based on some other implementation? Why for instance simple 'return GetCommandLineA()' is not enough as an initial implementation?
Probably I'm just stating obvious but the function skips executable name from the command line. That's why returning what GetCommandLineA returns is incorrect.
From what documentation/description is that obvious? That's not a common practice to send patches for not very obvious things without any tests. Why this implementation treats specially tabs and spaces but not other white space characters? Is that also obvious? How about the tests for quotes/embedded quotes?
-- Dmitry.
On 07/27/16 15:29, Dmitry Timoshkov wrote:
Piotr Caban <piotr.caban(a)gmail.com> wrote:
Once again: Where did you get an idea of such an implementation? Is this copied from some place or based on some other implementation? Why for instance simple 'return GetCommandLineA()' is not enough as an initial implementation?
Probably I'm just stating obvious but the function skips executable name from the command line. That's why returning what GetCommandLineA returns is incorrect.
From what documentation/description is that obvious? That's not a common practice to send patches for not very obvious things without any tests. Why this implementation treats specially tabs and spaces but not other white space characters? Is that also obvious? How about the tests for quotes/embedded quotes?
I was not asking Nikolay for the tests because I think it's not easy to write them. Some interesting tests needs executable name with ' ' in its name. I also don't like the idea of starting many processes just to test the function. On the other hand the initial test executable may be started in many different ways (like e.g. passing first argument quoted makes it impossible to check the return with simple comparison). That doesn't mean that I'm against adding a test or 2 for that. Anyway I think this implementation is good after doing some tests myself.
Piotr Caban <piotr.caban(a)gmail.com> wrote:
Once again: Where did you get an idea of such an implementation? Is this copied from some place or based on some other implementation? Why for instance simple 'return GetCommandLineA()' is not enough as an initial implementation?
Probably I'm just stating obvious but the function skips executable name from the command line. That's why returning what GetCommandLineA returns is incorrect.
From what documentation/description is that obvious? That's not a common practice to send patches for not very obvious things without any tests. Why this implementation treats specially tabs and spaces but not other white space characters? Is that also obvious? How about the tests for quotes/embedded quotes?
I was not asking Nikolay for the tests because I think it's not easy to write them. Some interesting tests needs executable name with ' ' in its name. I also don't like the idea of starting many processes just to test the function. On the other hand the initial test executable may be started in many different ways (like e.g. passing first argument quoted makes it impossible to check the return with simple comparison).
That doesn't mean that I'm against adding a test or 2 for that. Anyway I think this implementation is good after doing some tests myself.
Testing on your own is good, however that doesn't count like an answer to my questions above which still left unanswered. If Wine development would follow this kind of quality control it wouldn't get that far as it is now. -- Dmitry.
participants (2)
-
Dmitry Timoshkov -
Piotr Caban