28 Jul
2016
28 Jul
'16
11:08 a.m.
Piotr Caban <piotr(a)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.
--
Dmitry.
28 Jul
28 Jul
11:12 a.m.
New subject: [PATCH 2/2 v4] ucrtbase: Add _get_narrow_winmain_command_line tests
On 07/28/16 13:08, Dmitry Timoshkov wrote:
Piotr Caban <piotr(a)codeweavers.com> wrote:
CreateProcess will fail in case of '\r' or '\n' characters.
+ 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.
11:33 a.m.
New subject: [PATCH 2/2 v4] ucrtbase: Add _get_narrow_winmain_command_line tests
Piotr Caban <piotr(a)codeweavers.com> wrote:
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?
--
Dmitry.
+ 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.
11:45 a.m.
New subject: [PATCH 2/2 v4] ucrtbase: Add _get_narrow_winmain_command_line tests
On 07/28/16 13:33, Dmitry Timoshkov wrote:
Piotr Caban <piotr(a)codeweavers.com> wrote:
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).
+ 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.
1:27 p.m.
New subject: [PATCH 2/2 v4] ucrtbase: Add _get_narrow_winmain_command_line tests
Piotr Caban <piotr(a)codeweavers.com> wrote:
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.
--
Dmitry.
> +
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.
1:38 p.m.
New subject: [PATCH 2/2 v4] ucrtbase: Add _get_narrow_winmain_command_line tests
On 07/28/16 15:27, Dmitry Timoshkov wrote:
Piotr Caban <piotr(a)codeweavers.com> wrote:
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.
>> + 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?
1:43 p.m.
New subject: [PATCH 2/2 v4] ucrtbase: Add _get_narrow_winmain_command_line tests
Piotr Caban <piotr(a)codeweavers.com> wrote:
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.
--
Dmitry.
>>> +
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).
1:47 p.m.
New subject: [PATCH 2/2 v4] ucrtbase: Add _get_narrow_winmain_command_line tests
On 07/28/16 15:43, Dmitry Timoshkov wrote:
Piotr Caban <piotr(a)codeweavers.com> wrote:
'\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.
>>>> + 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.
3:25 p.m.
New subject: [PATCH 2/2 v4] ucrtbase: Add _get_narrow_winmain_command_line tests
Piotr Caban <piotr(a)codeweavers.com> wrote:
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?
--
Dmitry.
>>>>> + 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.
3:32 p.m.
New subject: [PATCH 2/2 v4] ucrtbase: Add _get_narrow_winmain_command_line tests
On 07/28/16 17:25, Dmitry Timoshkov wrote:
Piotr Caban <piotr(a)codeweavers.com> wrote:
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.
>>>>>> + 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.
3:36 p.m.
New subject: [PATCH 2/2 v4] ucrtbase: Add _get_narrow_winmain_command_line tests
Piotr Caban <piotr(a)codeweavers.com> wrote:
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.
>>>>>>> + 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.
2850
days inactive
2850
days old
10 comments
2 participants
participants (2)
-
Dmitry Timoshkov -
Piotr Caban