[PATCH 2/5] reg.exe: Add path_get_key() to remove boilerplate

[Hugh McMaster]

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