On Saturday, 8 Nov 2014 11:26:44 +0100, Jonathan Vollebregt wrote:
+static void reg_print_error(LSTATUS error_code) +{ + switch (error_code) + { + case ERROR_SUCCESS: + return; + case ERROR_BAD_COMMAND: + reg_message(STRING_INVALID_CMDLINE); + return; + default: + { + static const WCHAR error_string[] = {'%','0','5','d',':',' ','%','s',0}; + WCHAR *message = NULL; + FormatMessageW(FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_ALLOCATE_BUFFER, NULL, + error_code, MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), (WCHAR *)&message, 0, NULL); + + reg_message(STRING_ERROR); + reg_printfW(error_string, error_code, message); + LocalFree(message); + return; + } + } +}This seems like a step backwards to me. It would be much better to add the required functionality -- e.g. FormatMessage() -- and fix reg.exe printf-style functions in one go.
See http://source.winehq.org/patches/data/107578 for what I mean.
Also, mixing reg_message() and reg_printfW() just seems wrong. A clear resource string incorporating your formatting above would suffice, I expect.
-- Hugh McMaster
This seems like a step backwards to me. It would be much better to add the required functionality -- e.g. FormatMessage() -- and fix reg.exe printf-style functions in one go.
I'd still have to use reg_print_error as is because it's there to get the error string from standard error codes like ERROR_INVALID_DATA not to actually format the output string (FORMAT_MESSAGE_FROM_SYSTEM vs FORMAT_MESSAGE_FROM_STRING)
I like splitting the writing part into output_write (That might come in handy for printing long strings in reg query if we can't find a way to make reg_printfW variable size)
Giving reg_message extra arguments is a good idea, but I'm not sure if we're supposed to have format specifiers in LoadString resources.
I'm not sure whether having output_vprintf is really an advantage over just calling reg_printf since it uses different format specifiers than printf and basically does the same thing (Minus the variable size issue of course)
On Monday, 10 Nov 2014 14:46:49 +0100, Jonathan Vollebregt wrote:
Format specifiers are perfectly valid in resource strings. See programs/regsvr32/regsvr32.rc as one example.
Not having to deal with variable memory allocation is the obvious advantage of using output_vprintf(). My personal choice would be to convert static const WCHAR nonnumber[] and static const WCHAR unhandled[] to resource strings and using reg_message(). These messages really should be translated anyway.
That leaves the three static const WCHAR stubW[] arrays to consider. Personally, I would remove them. They serve no real purpose and native reg doesn't have anything like them.
As to the getting the error message from the system table, I came up with two possibilities.
static void __cdecl reg_printfW(const WCHAR* msg, ...) { __ms_va_list va_args;
__ms_va_start(va_args, msg); output_vprintf(msg, va_args); __ms_va_end(va_args); }
static void reg_print_error(LSTATUS error_code) { WCHAR *str, *buffer; int len, extra_char = 8; static const WCHAR error_string1[] = {'%','1','!','*','.','*','d','!',':',' ','%','3',0}; static const WCHAR error_string2[] = {'%','0','5','d',':',' ','%','s',0};
len = FormatMessageW(FORMAT_MESSAGE_FROM_SYSTEM|FORMAT_MESSAGE_ALLOCATE_BUFFER, NULL, error_code, 0, (LPWSTR)&str, 0, NULL); if (len == 0 && GetLastError() != NO_ERROR) { WINE_FIXME("Could not format system string: le=%u\n", GetLastError()); return; } reg_printfW(error_string1, 0, 5, error_code, str); /* Option 1 - fully automated */ /* Option 2 */ buffer = HeapAlloc(GetProcessHeap(), 0, (len + extra_char) * sizeof(WCHAR)); if (buffer) { sprintfW(buffer, error_string2, error_code, str); output_write(buffer, strlenW(buffer)); HeapFree(GetProcessHeap(), 0, buffer); } LocalFree(str); }
The reg patches have been languishing on the list for a while now.
As Hugh's example showed format specifiers are the preferred way of doing rc strings, it just seems a shame we have to duplicate so much.
Can we have 107580 committed so I can rebase on it?
I still wish there were a better way to deal with the fixed-size reg_printfW output but it looks like there is none.