[PATCH 2/5] reg.exe: Add path_get_key() to remove boilerplate
Hugh McMaster
hugh.mcmaster at outlook.com
Wed Sep 3 00:20:59 CDT 2014
Hi Jonathan,
On Monday, 1 September 2014, 15:05:31 +0200, Jonathan Vollebregt wrote:
+ CompareStringW(CP_ACP,NORM_IGNORECASE,
+ path,strlenW(short_HKEY_name[i]),
+ short_HKEY_name[i],strlenW(short_HKEY_name[i]))==2 ||
+ CompareStringW(CP_ACP,NORM_IGNORECASE,
+ path,strlenW(long_HKEY_name[i]),
+ long_HKEY_name[i],strlenW(long_HKEY_name[i]))==2)
1. CompareString does not accept code page identifiers. It accepts a locale.[1] You could use LOCAL_USER_DEFAULT or GetThreadLocale() instead.
2. Please use CSTR_EQUAL instead of 2.
3. You should also avoid calling strlenW four times.
int len_short_HKEY_name = strlenW(short_HKEY_name[i]); or something similar.
Alternatively, since wine/unicode.h is already being included, consider using strncmpiW.
4. In patches 2 to 5, please mimic the style of the surrounding/original code. That being said, if there are variations in style, just use the most common style.
+ if(path_get_rootkey_name(path))
+ {
+ return HKEYs[(path_get_rootkey_name(path) - &long_HKEY_name[0][0]) / 20];
+ }
+ else
+ {
+ return NULL;
+ }
Single line code in if/else blocks can be compacted to use less space. E.g.:
if (path_get_rootkey_name(path))
return HKEYs[(path_get_rootkey_name(path) - &long_HKEY_name[0][0]) / 20];
else
return NULL;
> +static const WCHAR * path_get_rootkey_name(LPWSTR path)
Asterisks should be placed next to the object name or type, e.g. static const WCHAR *path_get_rootkey_name(LPWSTR path).
> + if(!key)
> + if (RegQueryValueW(key,value_name,NULL,NULL)==ERROR_SUCCESS)
> + RegSetValueExW(key,value_name,0,reg_type,reg_data,reg_count);
Also, code is easier to read if there is space after keywords, arguments and around == (equivalent to). You have been slightly inconsistent throughout.
[1] http://msdn.microsoft.com/en-us/library/windows/desktop/dd317759%28v=vs.85%29.aspx
--
Hugh McMaster
More information about the wine-devel
mailing list