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;elsebreak;}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 '/'?