[PATCH v2 1/2] find.exe/tests: Add test for multiple files at once
Signed-off-by: Fabian Maurer <dark.shadow4(a)web.de> --- programs/find/tests/find.c | 42 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/programs/find/tests/find.c b/programs/find/tests/find.c index b8345034b5..76f023f5e8 100644 --- a/programs/find/tests/find.c +++ b/programs/find/tests/find.c @@ -264,6 +264,47 @@ static void run_find_unicode_(const BYTE *input, int input_len, const BYTE *out_ run_find_stdin_(wstr_quoted_test, input, input_len, out_expected_mangled, out_expected_mangled_len, exitcode_expected, file, line); } +static void run_find_file_multi(void) +{ + char path_temp_file1[MAX_PATH]; + char path_temp_file2[MAX_PATH]; + char path_temp_dir[MAX_PATH]; + HANDLE handle_file; + WCHAR commandline_new[MAX_PATH]; + char out_expected[500]; + const char* input = "ab\nbd"; + + GetTempPathA(ARRAY_SIZE(path_temp_dir), path_temp_dir); + GetTempFileNameA(path_temp_dir, "", 0, path_temp_file1); + GetTempFileNameA(path_temp_dir, "", 0, path_temp_file2); + handle_file = CreateFileA(path_temp_file1, GENERIC_WRITE, 0, NULL, CREATE_ALWAYS, 0, NULL); + write_to_handle(handle_file, (BYTE*)input, strlen(input)); + CloseHandle(handle_file); + handle_file = CreateFileA(path_temp_file2, GENERIC_WRITE, 0, NULL, CREATE_ALWAYS, 0, NULL); + write_to_handle(handle_file, (BYTE*)input, strlen(input)); + CloseHandle(handle_file); + + wsprintfW(commandline_new, L"\"b\" C:\\doesnotexist1 %hs C:\\doesnotexist1 %hs C:\\doesnotexist1", path_temp_file1, path_temp_file2); + + CharUpperA(path_temp_file1); + CharUpperA(path_temp_file2); + wsprintfA(out_expected, + "File not found - C:\\DOESNOTEXIST1\r\n" + "\r\n---------- %s\r\n" + "ab\r\nbd\r\n" + "File not found - C:\\DOESNOTEXIST1\r\n" + "\r\n---------- %s\r\n" + "ab\r\nbd\r\n" + "File not found - C:\\DOESNOTEXIST1\r\n", + path_temp_file1, path_temp_file2); + + todo_wine + run_find_stdin_(commandline_new, (BYTE*)"", 0, (BYTE*)out_expected, strlen(out_expected), 0, __FILE__, __LINE__); + + DeleteFileA(path_temp_file1); + DeleteFileA(path_temp_file2); +} + static void test_errors(void) { run_find_stdin_str("", "", "FIND: Parameter format not correct\r\n", 2); @@ -417,4 +458,5 @@ START_TEST(find) test_unicode_support_stdin(); test_file_search(); test_unicode_support_file(); + run_find_file_multi(); } -- 2.25.1
Signed-off-by: Fabian Maurer <dark.shadow4(a)web.de> --- v2: Add error check --- programs/find/find.c | 70 +++++++++++++++++++++++++++++++++----- programs/find/find.rc | 1 + programs/find/resources.h | 1 + programs/find/tests/find.c | 19 ----------- 4 files changed, 63 insertions(+), 28 deletions(-) diff --git a/programs/find/find.c b/programs/find/find.c index 34ca2801eb..c0175bbf27 100644 --- a/programs/find/find.c +++ b/programs/find/find.c @@ -147,7 +147,9 @@ int __cdecl wmain(int argc, WCHAR *argv[]) WCHAR *tofind = NULL; int i; int exitcode; - HANDLE input; + int file_paths_len = 0; + int file_paths_max = 1; + WCHAR** file_paths = heap_alloc(sizeof(WCHAR*)); TRACE("running find:"); for (i = 0; i < argc; i++) @@ -156,8 +158,6 @@ int __cdecl wmain(int argc, WCHAR *argv[]) } TRACE("\n"); - input = GetStdHandle(STD_INPUT_HANDLE); - for (i = 1; i < argc; i++) { if (argv[i][0] == '/') @@ -171,8 +171,12 @@ int __cdecl wmain(int argc, WCHAR *argv[]) } else { - FIXME("Searching files not supported yet\n"); - return 1000; + if (file_paths_len >= file_paths_max) + { + file_paths_max *= 2; + file_paths = heap_realloc(file_paths, sizeof(WCHAR*) * file_paths_max); + } + file_paths[file_paths_len++] = argv[i]; } } @@ -183,13 +187,61 @@ int __cdecl wmain(int argc, WCHAR *argv[]) } exitcode = 1; - while ((line = read_line_from_handle(input)) != NULL) + + if (file_paths_len > 0) + { + for (i = 0; i < file_paths_len; i++) + { + HANDLE input; + DWORD attributes = GetFileAttributesW(file_paths[i]); + WCHAR file_path_upper[MAX_PATH]; + + wcscpy(file_path_upper, file_paths[i]); + CharUpperW(file_path_upper); + + if (attributes == INVALID_FILE_ATTRIBUTES || (attributes & FILE_ATTRIBUTE_DIRECTORY)) + { + WCHAR buffer_message[64]; + WCHAR message[300]; + + LoadStringW(GetModuleHandleW(NULL), IDS_FILE_NOT_FOUND, buffer_message, ARRAY_SIZE(buffer_message)); + + wsprintfW(message, buffer_message, file_path_upper); + write_to_stdout(message); + continue; + } + + input = CreateFileW(file_paths[i], GENERIC_READ, 0, NULL, OPEN_EXISTING, 0, NULL); + if (!input) + { + ERR("Failed to open file %ls\n", file_paths[i]); + continue; + } + write_to_stdout(L"\r\n---------- "); + write_to_stdout(file_path_upper); + write_to_stdout(L"\r\n"); + while ((line = read_line_from_handle(input)) != NULL) + { + if (run_find_for_line(line, tofind)) + exitcode = 0; + + heap_free(line); + } + CloseHandle(input); + } + } + else { - if (run_find_for_line(line, tofind)) - exitcode = 0; + HANDLE input = GetStdHandle(STD_INPUT_HANDLE); + while ((line = read_line_from_handle(input)) != NULL) + { + if (run_find_for_line(line, tofind)) + exitcode = 0; - heap_free(line); + heap_free(line); + } } + heap_free(file_paths); return exitcode; } diff --git a/programs/find/find.rc b/programs/find/find.rc index 8c358c8677..b7376b6d11 100644 --- a/programs/find/find.rc +++ b/programs/find/find.rc @@ -22,4 +22,5 @@ STRINGTABLE { IDS_INVALID_PARAMETER "FIND: Parameter format not correct\r\n" IDS_INVALID_SWITCH "FIND: Invalid switch\r\n" + IDS_FILE_NOT_FOUND "File not found - %s\r\n" } diff --git a/programs/find/resources.h b/programs/find/resources.h index 2ebe197b7d..bdcc2802b5 100644 --- a/programs/find/resources.h +++ b/programs/find/resources.h @@ -23,5 +23,6 @@ #define IDS_INVALID_PARAMETER 1000 #define IDS_INVALID_SWITCH 1001 +#define IDS_FILE_NOT_FOUND 1002 #endif /* __WINE_FIND_RESOURCES_H */ diff --git a/programs/find/tests/find.c b/programs/find/tests/find.c index 76f023f5e8..85c15b12de 100644 --- a/programs/find/tests/find.c +++ b/programs/find/tests/find.c @@ -298,7 +298,6 @@ static void run_find_file_multi(void) "File not found - C:\\DOESNOTEXIST1\r\n", path_temp_file1, path_temp_file2); - todo_wine run_find_stdin_(commandline_new, (BYTE*)"", 0, (BYTE*)out_expected, strlen(out_expected), 0, __FILE__, __LINE__); DeleteFileA(path_temp_file1); @@ -313,7 +312,6 @@ static void test_errors(void) todo_wine /* Quotes are not properly passed into wine yet */ run_find_stdin_str("\"test", "", "FIND: Parameter format not correct\r\n", 2); run_find_stdin_str("\"test\" /XYZ", "", "FIND: Invalid switch\r\n", 2); - todo_wine run_find_stdin_str("\"test\" C:\\doesnotexist.dat", "", "File not found - C:\\DOESNOTEXIST.DAT\r\n", 1); } @@ -389,19 +387,12 @@ static void test_unicode_support_stdin(void) static void test_file_search(void) { - todo_wine run_find_file_str("\"\"", "test", "", 1); - todo_wine run_find_file_str("\"test\"", "", "", 1); - todo_wine run_find_file_str("\"test\"", "test", "test\r\n", 0); - todo_wine run_find_file_str("\"test\"", "test2", "test2\r\n", 0); - todo_wine run_find_file_str("\"test\"", "test\r2", "test\r2\r\n", 0); - todo_wine run_find_file_str("\"test2\"", "test", "", 1); - todo_wine run_find_file_str("\"test\"", "test\nother\ntest2\ntest3", "test\r\ntest2\r\ntest3\r\n", 0); } @@ -410,31 +401,21 @@ static void test_unicode_support_file(void) /* Test unicode support on files */ /* Test UTF-8 BOM */ - todo_wine run_find_file_unicode(str_en_utf8_nobom, str_en_utf8_nobom, 0); - todo_wine run_find_file_unicode(str_en_utf8_bom, str_en_utf8_bom, 0); /* Test russian characters */ - todo_wine run_find_file_unicode(str_rus_utf8_bom, str_rus_utf8_bom, 0); - todo_wine run_find_file_unicode(str_rus_utf8_nobom, str_rus_utf8_nobom, 0); /* Test japanese characters */ - todo_wine run_find_file_unicode(str_jap_utf8_nobom, str_jap_utf8_nobom, 0); - todo_wine run_find_file_unicode(str_jap_utf8_bom, str_jap_utf8_bom, 0); - todo_wine run_find_file_unicode(str_jap_shiftjis, str_jap_shiftjis, 0); /* Test unsupported encodings */ - todo_wine run_find_file_unicode(str_jap_utf16le_nobom, str_empty, 1); - todo_wine run_find_file_unicode(str_jap_utf16be_bom, str_empty, 1); - todo_wine run_find_file_unicode(str_jap_utf16be_nobom, str_empty, 1); /* Test utf16le */ -- 2.25.1
Fabian Maurer <dark.shadow4(a)web.de> writes:
+ HANDLE input; + DWORD attributes = GetFileAttributesW(file_paths[i]); + WCHAR file_path_upper[MAX_PATH]; + + wcscpy(file_path_upper, file_paths[i]); + CharUpperW(file_path_upper); + + if (attributes == INVALID_FILE_ATTRIBUTES || (attributes & FILE_ATTRIBUTE_DIRECTORY)) + { + WCHAR buffer_message[64]; + WCHAR message[300]; + + LoadStringW(GetModuleHandleW(NULL), IDS_FILE_NOT_FOUND, buffer_message, ARRAY_SIZE(buffer_message)); + + wsprintfW(message, buffer_message, file_path_upper); + write_to_stdout(message); + continue; + } + + input = CreateFileW(file_paths[i], GENERIC_READ, 0, NULL, OPEN_EXISTING, 0, NULL); + if (!input) + { + ERR("Failed to open file %ls\n", file_paths[i]); + continue; + }
Again, there's no reason to check the attributes first. Just try to open the file and handle the failure. -- Alexandre Julliard julliard(a)winehq.org
Hello Alexandre,
Again, there's no reason to check the attributes first. Just try to open the file and handle the failure.
Sorry, I misunderstood. I thought it was best practice to check if it exists before trying to open it. Regards, Fabian Maurer
Fabian Maurer <dark.shadow4(a)web.de> writes:
Hello Alexandre,
Again, there's no reason to check the attributes first. Just try to open the file and handle the failure.
Sorry, I misunderstood. I thought it was best practice to check if it exists before trying to open it.
No, that's actually worst practice. Just because it existed at the time you checked doesn't mean it can be opened: it could have been removed in the meantime, or not have enough permissions, or a million other possible errors. You always need to do a proper failure check at open time, so checking beforehand is only a waste of time. -- Alexandre Julliard julliard(a)winehq.org
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=65379 Your paranoid android. === w1064v1809_ja (32 bit report) === Report validation errors: find.exe:find is missing some failure messages === w1064v1809_zh_CN (32 bit report) === Report validation errors: find.exe:find is missing some failure messages
=== w1064v1809_ja (32 bit report) ===
Report validation errors: find.exe:find is missing some failure messages
=== w1064v1809_zh_CN (32 bit report) ===
Report validation errors: find.exe:find is missing some failure messages
What exactly does that mean? Did I write something bad into the log? Regards, Fabian Maurer
On Tue, 18 Feb 2020, Fabian Maurer wrote:
=== w1064v1809_ja (32 bit report) ===
Report validation errors: find.exe:find is missing some failure messages
=== w1064v1809_zh_CN (32 bit report) ===
Report validation errors: find.exe:find is missing some failure messages
What exactly does that mean? Did I write something bad into the log?
The TestBot does not recognize these lines: find.c:315: Test failed: [...] find.c:423: Test marked todo: The reason is it strips the end-of-line spaces to deal with the CR / LF variations but because there's no text after the colon it ends up removing the space that follows that colon and then it does not recognize these lines. I'll fix that. -- Francois Gouget <fgouget(a)codeweavers.com>
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=65378 Your paranoid android. === w1064v1809_ja (32 bit report) === Report validation errors: find.exe:find is missing some failure messages === w1064v1809_zh_CN (32 bit report) === Report validation errors: find.exe:find is missing some failure messages
participants (4)
-
Alexandre Julliard -
Fabian Maurer -
Francois Gouget -
Marvin