Re: [gdi32 1/6]: Cleanup the update_font_list().

[Huw Davies]

On Tue, Oct 03, 2006 at 01:39:23PM +0900, Byeong-Sik Jeon wrote:
> Cleanup the update_font_list().

I'm not convinced this is a cleanup, see below for comments.

> +#define AddNlsFontToRegistry(hKey, ValueNameA, Data, bForce) do\
> +    {\
> +        DWORD reg_type;\
> +        if(bForce == TRUE ||\
> +           RegQueryValueExA(hKey, ValueNameA, 0, &reg_type, NULL, NULL) != 
> ERROR_SUCCESS ||\
> +           reg_type != REG_SZ)\
> +        {\
> +            RegSetValueExA(hKey, ValueNameA, 0, REG_SZ, (const BYTE *)Data, 
> (DWORD)(strlen(Data) + 1));\
> +        }\
> +    } while(0)

Please avoid macro abuse like this and use an inlined function
instead.  It's also nicer to call it something like
add_nls_font_to_registry so that it's easier to read and doesn't look
like a Windows API.

> +    RegCreateKeyExW(HKEY_LOCAL_MACHINE,
> +                    is_win9x() ? win9x_font_reg_key : winnt_font_reg_key,
> +                    0, NULL, 0, KEY_ALL_ACCESS, NULL, &hkey, NULL);
> +    AddNlsFontToRegistry(hkey, "Courier"      , 
> matched_nls_font_list->courier, TRUE);
> +    AddNlsFontToRegistry(hkey, "MS Serif"     , 
> matched_nls_font_list->serif,   TRUE);
> +    AddNlsFontToRegistry(hkey, "MS Sans Serif", 
> matched_nls_font_list->sserif,  TRUE);
> +    AddNlsFontToRegistry(hkey, "Small Fonts"  , 
> matched_nls_font_list->small,   TRUE);
> +    RegCloseKey(hkey);

Here you only create either the win9x key or the winnt key.  I'd
prefer that you created both like the old code did.

Thanks,
Huw.
-- 
Huw Davies
[EMAIL PROTECTED]