[PATCH] find: First simple implementation and tests
Nikolay Sivov
nsivov at codeweavers.com
Sat Sep 15 13:42:46 CDT 2018
On 09/15/2018 06:42 PM, Fabian Maurer wrote:
> The message text is the same as on windows,
> since the exit code doesn't seem reliable.
Is it unreliable across Windows versions? I only see error message for
invalid input, not for the case when string was or wasn't found. So why
does it matter?
> +BOOL read_char_from_handle(HANDLE handle, char *char_out)
Function name does not correspond with what function is doing.
> +{
> + static char buffer[4096];
> + static UINT buffer_max = 0;
> + static UINT buffer_pos = 0;
What's a reason for this to be static?
> +
> + /* Read next content into buffer */
> + if (buffer_pos >= buffer_max)
> + {
> + BOOL success = ReadFile(handle, buffer, 4096, &buffer_max, NULL);
> + if (!success)
Separate variable does not seem to be necessary.
> + return FALSE;
> + buffer_pos = 0;
> + }
> +
> + *char_out = buffer[buffer_pos++];
> + return TRUE;
> +}
> +/* Reads a line from a handle, returns TRUE if there is more to be read */
> +BOOL read_line_from_handle(HANDLE handle, WCHAR **line_out)
> +{
> + int buffer_size = 4096;
> + char c;
> + int length = 0;
> + WCHAR *line_converted;
> + int line_converted_length;
> + BOOL success;
> + char *line = heap_alloc(buffer_size);
> +
> + for (;;)
> + {
> + success = read_char_from_handle(handle, &c);
> +
> + /* Check for EOF */
> + if (!success)
> + {
> + if (length == 0)
> + return FALSE;
> + else
> + break;
> + }
> +
> + if (c == '\n')
> + break;
> +
> + /* Make sure buffer is large enough */
> + if (length + 1 >= buffer_size)
> + {
> + buffer_size *= 2;
> + line = heap_realloc(line, buffer_size);
> + }
> +
> + line[length++] = c;
> + }
> +
> + line[length] = 0;
> + if (length - 1 >= 0 && line[length - 1] == '\r')
> + line[length - 1] = 0;
> +
> + line_converted_length = MultiByteToWideChar(CP_ACP, 0, line, -1, 0, 0);
> + line_converted = heap_alloc(line_converted_length * sizeof(WCHAR));
> + MultiByteToWideChar(CP_ACP, 0, line, -1, line_converted, line_converted_length);
> +
> + heap_free(line);
> +
> + *line_out = line_converted;
> + return TRUE;
> +}
CP_ACP is probably not correct in this case.
> +void write_to_handle(HANDLE handle, const WCHAR *str)
> +{
> + DWORD bytes_written_sum = 0;
> + char *str_converted;
> + UINT str_converted_length;
> + UINT str_length = lstrlenW(str);
> +
> + str_converted_length = WideCharToMultiByte(CP_ACP, 0, str, str_length, NULL, 0, NULL, NULL);
> + str_converted = heap_alloc(str_converted_length);
> + WideCharToMultiByte(CP_ACP, 0, str, str_length, str_converted, str_converted_length, NULL, NULL);
> +
> + do
> + {
> + DWORD bytes_written;
> + WriteFile(handle, str_converted, str_converted_length, &bytes_written, NULL);
> + bytes_written_sum += bytes_written;
> + } while (bytes_written_sum < str_converted_length);
> +
> + heap_free(str_converted);
> +}
> +
> +void find_printf(const WCHAR *str)
> +{
> + write_to_handle(GetStdHandle(STD_OUTPUT_HANDLE), str);
> +}
Check how it's done in reg/regedit, probably you should reuse it here.
Less important, but why two helpers here? And why do you need a loop to
write this buffer?
> +BOOL run_find_for_line(const WCHAR *line, const WCHAR *tofind)
> +{
> + void *found;
> + WCHAR lineending[] = {'\r', '\n', 0};
> +
> + if (lstrlenW(line) == 0)
> + return FALSE;
> +
> + found = StrStrW(line, tofind);
> +
> + if (found)
> + {
> + find_printf(line);
> + find_printf(lineending);
> + return TRUE;
> + }
> +
> + return FALSE;
> +}
There's strstrW(), importing shlwapi only for that is too much.
> + WCHAR message_parameter_invalid[64];
> + WCHAR message_switch_invalid[64];
> int i;
>
>
> + LoadStringW(GetModuleHandleW(NULL), IDS_INVALID_PARAMETER, message_parameter_invalid, sizeof(message_parameter_invalid));
> + LoadStringW(GetModuleHandleW(NULL), IDS_INVALID_SWITCH, message_switch_invalid, sizeof(message_switch_invalid));
You don't need them both at the same time. Also check the arguments.
> + input = GetStdHandle(STD_INPUT_HANDLE);
> +
> + for (i = 1; i < argc; i++)
> + {
> + if (argv[i][0] == '/')
> + {
> + find_printf(message_switch_invalid);
> + return 2;
> + }
> + else if(tofind == NULL)
> + {
> + tofind = argv[i];
> + }
> + }
> +
> + if (tofind == NULL)
> + {
> + find_printf(message_parameter_invalid);
> + return 2;
> + }
> +
> + exitcode = 1;
> + while (read_line_from_handle(input, &line))
> + {
> + if (run_find_for_line(line, tofind))
> + exitcode = 0;
> +
> + heap_free(line);
> + }
I don't understand why you're reading from stdin uncoditionally. This
utility takes options, string to find, and a list of file paths. Only
when paths are not provided it should read from stdin. Will this work if
you have your string argument starts with '/'?
More information about the wine-devel
mailing list