[PATCH 3/7] reg: Add path/key conversion functions

[Jonathan Vollebregt]

On 11/04/2014 10:48 PM, Stefan Dösinger wrote:
> My reading comprehension may be lacking again, but can't you just call RegDeleteTree(subkey, NULL)? You're also assuming the caller isn't trying to nuke HKEY_LOCAL_MACHINE:-)  .

That deletes everything below the key, but not the key itself, so I have 
to do it that way.

>> +static BOOL sane_path(const WCHAR *key)
>> +{
>> +    int i = strlenW(key);
>> +
>> +    if (i < 3 || (key[i - 1] == '\\' && key[i - 2] == '\\'))
>> +    {
>> +        reg_print_error(ERROR_BADKEY);
>> +        return FALSE;
>> +    }
> I think I have asked this a few times, please forgive me if I forgot the answer: Did you test if RegAddKey and friends handle this for you?
> dlls/advapi32/tests/registry.c would profit from some tests for the error cases you have in programs/reg/tests/reg.c .

Reg is supposed to throw an error if the key ends in more than one 
backslash, RegCreateKey/RegOpenKey doesn't so it has to be checked manually.

>> +#define ERROR_INVALID_TYPE      20001
> What does RegSetValueEx return if you pass an invalid type? The API makes this error condition possible, so there's likely a return value for it. (Yeah, I know, it's not your fault advapi32/tests has no test for this. If I understand server/registry.c correctly such a call succeeds and creates a blob with an undefined type. Still needs tests)
>
> ERROR_INVALID_DATATYPE may be a candidate.

The message with ERROR_INVALID_DATATYPE says it indicates the *data* has 
the wrong type, nothing about the type itself.

Given an incorrect type RegSetValueEx returns ERROR_ACCESS_DENIED, which 
isn't the most descriptive of error codes. Perhaps it would still be 
better to make our own more descriptive error code

> Another possibility is that native assumes a default value string of "" if /d is not specified, which then produces behavior like an empty REG_SZ and fails to convert into a DWORD.

Nice catch! That fits perfectly! 29 loc less!

>> +        default:
>> +            return ERROR_UNSUPPORTED_TYPE;
> I think this is a bad place to catch unsupported data types. "type" comes from one of your own functions, namely wchar_get_type(). If reg.exe shouldn't support any of the data types not explicitly handled here then wchar_get_type should already return an error in this case. Otherwise you can make this a FIXME("Add support for type %u\n" type);

What should I do in default then? It would be a bad idea to return 
ERROR_SUCCESS