Re: ntdll: Remove more path trailing chars.

[Nikolay Sivov]

On 4/2/2011 10:51, Vincent Pelletier wrote:
> Add tests via FindFirstFileA.
> Fixes bug 22635.
>
> dlls/kernel32/tests/file.c |  119 ++++++++++++++++++++++++++++++++++++++++++++
>   dlls/ntdll/path.c          |    8 +++-
>   2 files changed, 126 insertions(+), 1 deletions(-)
>
> diff --git a/dlls/kernel32/tests/file.c b/dlls/kernel32/tests/file.c
> index 4e8136a..dae8eee 100644
> --- a/dlls/kernel32/tests/file.c
> +++ b/dlls/kernel32/tests/file.c
> @@ -2164,6 +2164,125 @@ static void test_FindFirstFileA(void)
>       err = GetLastError();
>       ok ( handle == INVALID_HANDLE_VALUE, "FindFirstFile on %s should Fail\n", buffer2 );
>       ok ( err == ERROR_PATH_NOT_FOUND, "Bad Error number %d\n", err );
> -
> - /* try FindFirstFileA on "test-dir" with forbidden chars */
> - CreateDirectoryA("test-dir", NULL);
> - _lclose(_lcreat("test-file", 0));
> -
>
> +#define FindFirstFileA_ok(value, caption, todo) { \
> -
>    if(todo) { \
> -
>        todo_wine ok(value, caption);\
> -
>    } else { \
> -
>        ok(value, caption);\
> -
>    } \
> - }
> -
>
> +#define test_FindFirstFileA_with(x, y, todo) { \
> -
>    handle = FindFirstFileA(x,&data);\
> -
>    if (y) { \
> -
>        FindFirstFileA_ok( \
> -
>            handle != INVALID_HANDLE_VALUE, \
> -
>            "FindFirstFile on "x" should succeed\n", \
> -
>            todo);\
> -
>    } else { \
> -
>        FindFirstFileA_ok( \
> -
>            handle == INVALID_HANDLE_VALUE, \
> -
>            "FindFirstFile on "x" should fail\n", \
> -
>            todo);\
> -
>    } \
> -
>    if (handle != INVALID_HANDLE_VALUE) \
> -
>         ok( FindClose(handle) == TRUE, "Failed to close handle "x"\n"); \
> - }
> -
> - /* Disallowed chars being ignored: */
> - test_FindFirstFileA_with("test-dir/.", 1, 0);
> - test_FindFirstFileA_with("test-dir\.", 1, 0);
> - test_FindFirstFileA_with("test-dir<.", 1, 0);
> - test_FindFirstFileA_with("test-dir<  ", 1, 0);
> - test_FindFirstFileA_with("test-dir>.", 1, 0);
> - test_FindFirstFileA_with("test-dir>  ", 1, 0);
> - test_FindFirstFileA_with("test-dir".", 1, 0);
> - test_FindFirstFileA_with("test-dir" ", 1, 0);
> - test_FindFirstFileA_with("test-dir/>", 1, 0);
> - test_FindFirstFileA_with("test-dir/<", 1, 0);
> - test_FindFirstFileA_with("test-dir/"", 1, 0);
> - test_FindFirstFileA_with("test-dir<>", 1, 0);
> - test_FindFirstFileA_with("test-dir<<", 1, 0);
> - test_FindFirstFileA_with("test-dir<"", 1, 0);
> - test_FindFirstFileA_with("test-dir>>", 1, 0);
> - test_FindFirstFileA_with("test-dir><", 1, 0);
> - test_FindFirstFileA_with("test-dir>"", 1, 0);
> - test_FindFirstFileA_with("test-dir">", 1, 0);
> - test_FindFirstFileA_with("test-dir"<", 1, 0);
> - test_FindFirstFileA_with("test-dir""", 1, 0);
> - test_FindFirstFileA_with("test-dir\<", 1, 0);
> - test_FindFirstFileA_with("test-dir\>", 1, 0);
> - test_FindFirstFileA_with("test-dir\"", 1, 0);
> - /* But space prevents forward&  back slashes from being stripped */
> - test_FindFirstFileA_with("test-dir<", 0, 1);
> - test_FindFirstFileA_with("test-dir/ ", 0, 0);
> - test_FindFirstFileA_with("test-dir/ .", 0, 0);
> - test_FindFirstFileA_with("test-dir/. ", 0, 1);
> - test_FindFirstFileA_with("test-dir\ ", 0, 0);
> - test_FindFirstFileA_with("test-dir\ .", 0, 0);
> - test_FindFirstFileA_with("test-dir\. ", 0, 1);
> - test_FindFirstFileA_with("test-dir/. ", 0, 1);
> - test_FindFirstFileA_with("test-dir\. ", 0, 1);
> - /* But forward&  back slashes are not tolerated on a file */
> - test_FindFirstFileA_with("test-file/>", 0, 1);
> - test_FindFirstFileA_with("test-file/<", 0, 1);
> - test_FindFirstFileA_with("test-file/"", 0, 1);
> - test_FindFirstFileA_with("test-file<>", 1, 0);
> - test_FindFirstFileA_with("test-file<<", 1, 0);
> - test_FindFirstFileA_with("test-file<"", 1, 0);
> - test_FindFirstFileA_with("test-file>>", 1, 0);
> - test_FindFirstFileA_with("test-file><", 1, 0);
> - test_FindFirstFileA_with("test-file>"", 1, 0);
> - test_FindFirstFileA_with("test-file">", 1, 0);
> - test_FindFirstFileA_with("test-file"<", 1, 0);
> - test_FindFirstFileA_with("test-file""", 1, 0);
> - test_FindFirstFileA_with("test-file\<", 0, 1);
> - test_FindFirstFileA_with("test-file\>", 0, 1);
> - test_FindFirstFileA_with("test-file\"", 0, 1);
> - /* Even when many */
> - test_FindFirstFileA_with("test-dir"""""", 1, 0);
> - test_FindFirstFileA_with("test-dir<<<><><>""""<<<>", 1, 0);
> - /* But only in the end of path */
> - test_FindFirstFileA_with("test-/>dir", 0, 0);
> - test_FindFirstFileA_with("/>test-dir", 0, 0);
> - test_FindFirstFileA_with(">test-dir", 0, 0);
> - test_FindFirstFileA_with(">>test-dir", 0, 0);
> - test_FindFirstFileA_with(">test->dir", 0, 0);
> - test_FindFirstFileA_with("><test->dir", 0, 0);
> - test_FindFirstFileA_with("<test->dir", 0, 0);
> - test_FindFirstFileA_with("<"test->dir", 0, 0);
> -
> - /* Slash after ignored chars is not ignored: */
> - test_FindFirstFileA_with("test-dir//", 0, 0);
> - test_FindFirstFileA_with("test-dir</", 0, 0);
> - test_FindFirstFileA_with("test-dir>/", 0, 0);
> - test_FindFirstFileA_with("test-dir"/", 0, 0);
> - test_FindFirstFileA_with("test-dir\", 0, 0);
> - test_FindFirstFileA_with("test-dir\\", 0, 0);
> - test_FindFirstFileA_with("test-dir\/", 0, 0);
> - test_FindFirstFileA_with("test-dir/\", 0, 0);
> -
> - /* Disalowed chars being preserved: */
> - test_FindFirstFileA_with("test-dir/:", 0, 0);
> - test_FindFirstFileA_with("test-dir/|", 0, 0);
> -
> - /* Slashes converted to backslashes */
> - test_FindFirstFileA_with("test-dir/", 0, 0);
> - test_FindFirstFileA_with("./test-dir", 1, 0);
> - test_FindFirstFileA_with("./test-dir/", 0, 0);
> - test_FindFirstFileA_with("test-dir\", 0, 0);
> - test_FindFirstFileA_with(".\test-dir", 1, 0);
> - test_FindFirstFileA_with(".\test-dir\", 0, 0);
> -
>
> +#undef test_FindFirstFileA_with
> +#undef FindFirstFileA_ok
> - DeleteFileA("test-file");
> - RemoveDirectoryA("test-dir");
> }

Please use table for test data and put your macro body in a loop.

ntdll: Remove more path trailing chars.

[Vincent Pelletier]

Hi.

Le samedi 02 avril 2011 09:16:53, Nikolay Sivov a écrit :
> Please use table for test data and put your macro body in a loop.

Done locally (indeed nicer).
I have one more thing to figure out before posting again: how to deal with the 
NT4 failures testbot detected[1].

Those tests are boolean, so I doubt an "ok(pass || fail /* NT4 */)" makes any 
sense.
I saw that it is discouraged (forbidden ?) to depend on OS version in tests, 
so I will avoid that path unless asked to follow it.
I've been advised on IRC to use "broken()", but I don't see how to use it for 
such boolean case (in my understanding it has a meaning if different error 
status a returned by different windows versions, not between a success and an 
error status).

[1] http://testbot.winehq.org/JobDetails.pl?Key=10288#k201

Regards,

ntdll: Remove more path trailing chars.

[Juan Lang]

Hi Vincent,
> Those tests are boolean, so I doubt an "ok(pass || fail /* NT4 */)" makes any
> sense.
> I saw that it is discouraged (forbidden ?) to depend on OS version in tests,
> so I will avoid that path unless asked to follow it.
> I've been advised on IRC to use "broken()", but I don't see how to use it for
> such boolean case (in my understanding it has a meaning if different error
> status a returned by different windows versions, not between a success and an
> error status).

It does work for a boolean case too.  You'd want to use "ok(pass ||
broken(fail /* NT4 */)".
--Juan

ntdll: Remove more path trailing chars.

[Nikolay Sivov]

On 4/5/2011 23:36, Vincent Pelletier wrote:
> Hi.
>
> Le samedi 02 avril 2011 09:16:53, Nikolay Sivov a écrit :
> > Please use table for test data and put your macro body in a loop.
>
> Done locally (indeed nicer).
> I have one more thing to figure out before posting again: how to deal with the
> NT4 failures testbot detected[1].
>
> Those tests are boolean, so I doubt an "ok(pass || fail /* NT4 */)" makes any
> sense.
> I saw that it is discouraged (forbidden ?) to depend on OS version in tests,
> so I will avoid that path unless asked to follow it.
> I've been advised on IRC to use "broken()", but I don't see how to use it for
> such boolean case (in my understanding it has a meaning if different error
> status a returned by different windows versions, not between a success and an
> error status).

It depends on test. I see two ways at least - use broken() as 
alternative result always when it should fail or
add another test data field named like broken_ret for example, and 
compare with it.

It's discouraged to explicitly check for OS version, if some feature is 
missing in earlier versions
you should detect that and win_skip() for it.
> [1] http://testbot.winehq.org/JobDetails.pl?Key=10288#k201
>
> Regards,