[PATCH 1/6] reg: Sanitize key paths in main

[Jonathan Vollebregt]

> Inconsistent curly bracket placement. The other patches have more cases of that.

Should the arrays also have the opening brace on it's own line? A quick 
grep shows both styles for arrays in wine.

>> +        else
>> +            reg_message(STRING_INVALID_KEY);
> Doesn't get_rootkey() implicitly take care of this? I don't see why you need an extra check for a single leading backslash.
>
> There's one (intended?) side effect of this check though: It ensures that the code below doesn't crash on the input string "\\".

The tests show that having any backslash at the start results in an 
error. That particular if just picks which error message to show.

>> +    i = strlenW(key);
>> +    if (key[i - 1] == '\\')
>> +        key[i - 1] = 0;
> Why do you need this? The existing code and the todo_wine test suggests that RegCreateKey accepts any number of trailing slashes. Adding a test for this in dlls/advapi32/tests/registry.c would be a good idea.

reg/tests/reg.c:109 shows only the winXP version accepts multiple 
trailing backslashes.

The reason for stripping a single trailing backslash was for output 
consistency with query (As yet unimplemented) Specifically whether query 
outputs "HKEY_CURRENT_USER\\key" or "HKEY_CURRENT_USER\\key\\" as the 
key name.

Should I still make it const and handle the backslash stripping in a 
different function?

>> +static inline int path_rootname_cmp(const WCHAR *path, const WCHAR *name)
>> +{
>> +    DWORD length = strlenW(name);
>> +
>> +    return (strncmpiW(path, name, length) == 0 && (path[length] == 0 || path[length] == '\\'));
>> +}
> I don't like the names "path" and "name". I wondered why you're relying on the length of name. A better name for "path" could be "input", but I can't think of anything for "name".
>
Because the input is the full path we have to stop comparing after the 
root key or it will always return 0 unless it's passed a root key.

I think the "path" parameter name makes this more obvious, but I could 
throw a comment in if that helps.

>> +    path = strchrW(path, '\\');
>> +    if (!path)
>> +        return k;
> I see that this is why you need the trailing backslash elimination in the previous patch. But still, do you need this? What happens when you call e.g. RegOpenKey(HKEY_CURRENT_USER, "\\", &k)? If it returns HKEY_CURRENT_USER or a handle that points to HKEY_CURRENT_USER you're fine. Similarly for RegOpenKey(HKEY_CURRENT_USER, "", &k).

Actually this solves 2 separate problems. RegOpenKey takes the path 
after the root key but the path provided includes the root key, so 
strchrW skips the root key in the path in advance of passing it to 
RegOpenKey.

On the other hand, if someone inputs only a root key IE "HKCU" it will 
return NULL. We already have the key from path_get_rootkey, so we just 
return it here.