[PATCH 1/7] regedit: Use a helper for LoadString() and add debug information

[Hugh McMaster]

On Sunday, 29 January 2017 11:23 PM, Dmitry Timoshkov wrote:    
>Hugh McMaster wrote:
>> -static INT vmessagebox(HWND hwnd, INT buttons, INT titleId, INT resId, va_list ap)
>> +static void load_string(unsigned int msg_id, WCHAR *buf, unsigned int count)
>>  {
>>      static const WCHAR errorW[] = {'E','r','r','o','r',0};
>> -    static const WCHAR unknownW[] = {'U','n','k','n','o','w','n',' ','e','r','r','o','r',' ','s','t','r','i','n','g','!',0};
>>  
>> +    if (!buf) return;
>> +
>> +    if (!LoadStringW(hInst, msg_id, buf, count))
>> +    {
>> +        WINE_FIXME("LoadString failed with %u\n", GetLastError());
>> +        lstrcpyW(buf, errorW);
>> +    }
>> +}
>
> Probably using a flavour of lstrcpy that checks the target buffer length
> would be more appropriate. Although simply returning an empty buffer and
> an error indicator to the caller could be slightly better. Under which
> conditions LoadString may fail here?

The buffer size must be at least six WCHARs to allow for errorW, so we could 
just do something like if (!buf || count < 6) return;

In this code, LoadString() could fail if the resource ID doesn't exist
or if count is 0 or 1. But since we are controlling all of the code, none of this
is likely.

For MessageBox captions, if we pass in NULL, the default string "Error" 
is loaded by user32. So, apart from a few instances, we wouldn't have to 
handle captions at all. But we would have to do something if LoadString() 
failed for the body text, otherwise the user would see a blank message box.

The original code was:
if (!LoadStringW(hInst, titleId, title, COUNT_OF(title))) lstrcpyW(title, errorW);
if (!LoadStringW(hInst, resId, errfmt, COUNT_OF(errfmt))) lstrcpyW(errfmt, unknownW);

Do you think it is worth replacing? My main reason for adding load_string() was to avoid
duplicating the FIXME line and to avoid "Unknown error string".

--
Hugh