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 '/'?