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, ®_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
huw(a)codeweavers.com