[PATCH 2/2 v4] ucrtbase: Add _get_narrow_winmain_command_line tests

Dmitry Timoshkov dmitry at baikal.ru
Thu Jul 28 10:36:23 CDT 2016


Piotr Caban <piotr at codeweavers.com> wrote:

> >>>>>>>>>> +    sprintf(cmd, "\"\"%c\"\"\"%s\" \t \"misc\" cmd", name[0], name+1);
> >>>>>>>>>> +    memset(&startup, 0, sizeof(startup));
> >>>>>>>>>> +    startup.cb = sizeof(startup);
> >>>>>>>>>> +    CreateProcessA(path, cmd, NULL, NULL, TRUE,
> >>>>>>>>>> +            CREATE_DEFAULT_ERROR_MODE|NORMAL_PRIORITY_CLASS,
> >>>>>>>>>> +            NULL, NULL, &startup, &proc);
> >>>>>>>>>> +    winetest_wait_child_process(proc.hProcess);
> >>>>>>>>>
> >>>>>>>>> Thanks for adding the tests. It shouldn't be too hard to also test other
> >>>>>>>>> white space characters, at least testing '\r' and '\n' would be nice to
> >>>>>>>>> have.
> >>>>>>>>>
> >>>>>>>> CreateProcess will fail in case of '\r' or '\n' characters.
> >>>>>>>
> >>>>>>> In a simple test CreateProcess() works just fine here under Windows 7 when a
> >>>>>>> passed in command line contains '\r' and/or '\n', where does it fail for you?
> >>>>>>>
> >>>>>> I'll try to be a little more precise - '\r' is treated as another
> >>>>>> argument so it can't be used before first argument (misc in this case).
> >>>>>
> >>>>> Then it clearly deserves an additional test case. And a test for '\n' since
> >>>>> it doesn't break anything.
> >>>> '\n' behaves the same as '\r' according to my test.
> >>>>
> >>>> I think I have already explained you while '\r' case can't be tested in
> >>>> reasonable way.
> >>>
> >>> I don't see why '\r' and '\n' can't be tested. If CreateProcess() call doesn't
> >>> fail, what you need to test is the resulting command line that the child process
> >>> receives. It's clear and simple.
> >>>
> >> I've already wrote you that '\r' is treated as argument - because of
> >> that it can't be added before "misc" argument (the test executable needs
> >> first argument to be "misc"). The function
> >> _get_narrow_winmain_command_line only interprets part of the command
> >> line before first argument.
> >
> > How about adding '\r' after "misc" or at whatever place you think appropriate
> > and simply test both the resulting command line and the result of narrow one?
> > Just execute the process and test whatever it gets as the command line and
> > the narrow one? What is so hard with that?
> >
> Such test will be useless. It's already shown that 
> _get_narrow_winmain_command_line returns substring of GetCommandLineA. 
> It also doesn't touch anything past the beginning of first argument.

There is no such a thing as a useless test IMHO, any test exercises
the API and shows its result preventing possible regressions in future.

-- 
Dmitry.



More information about the wine-devel mailing list