Re: [PATCH 6/8] reg: Add wchar/raw data conversion functions

[Stefan Dösinger]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Am 2014-11-06 22:17, schrieb Jonathan Vollebregt:
> +static LSTATUS wchar_get_data(  const WCHAR *input,     const DWORD type,
> -
>                            const WCHAR separator,  DWORD *size_out,    BYTE **out)

This whitespace use is a bit weird. Is there a precedent for it?
> -
>        ** (DWORD **) out = i;

The space between ** and ( is a bit odd too.
> -
>                /* If it's the first character or the previous one was a separator */
> -
>                if (!p || temp[p - 1] == 0)
> -
>                {
> -
>                    HeapFree(GetProcessHeap(), 0, temp);
> -
>                    return ERROR_BAD_COMMAND;
> -
>                }

What does advapi32 do if you have empty substrings? (I.e., the same question as with the trailing backslashes: Does this code belong into reg.exe or advapi32.dll?) If advapi32 returns an error you can probably use it instead of the internal ERROR_NOT_INT_OR_NEG.
> -
>        *out = HeapReAlloc(GetProcessHeap(), 0, temp, p * sizeof(WCHAR));

Technically this could fail. But I don't think it is necessary - the compiled code for the HeapReAlloc invocation takes probably more space than the few bytes you're freeing a little bit earlier.
> -
>        return ERROR_SUCCESS;
> -
>    }
> -
>    case REG_BINARY:
> -
>    {
> -
>        BYTE * temp = HeapAlloc(GetProcessHeap(), 0, strlenW(input));

Inconsistent style.
> -
>    case REG_BINARY:
>
> ...
> -
>                return ERROR_BAD_COMMAND;

Does native write different error messages for a negative DWORD, improperly separated REG_MULTI_SZ and invalid REG_BINARY?
> -
>        *out = HeapReAlloc(GetProcessHeap(), 0, temp-odd, p);

MSDN says the third parameter has to be a pointer returned by HeapAlloc or HeapReAlloc.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJUXLpiAAoJEN0/YqbEcdMwEB4P/R4usEhK7b6woRAHZWXh3oSg
UfhpgUpQQELq0nw2WMQrVtkhwvcJywSAkaI/MPyB0vDR6DScqTrcDe2j78yIyMiY
DWJ24q+Xfix84fiOfSKbMWJZMAcZnCfn0pliwN/TjfzjOLdAZc6B0fO84wEJfHLL
DstJzYgzANg1u3FZJtNpAgex2PpU8MpyATXqbvoZdPhJO3MCwuD9KVzqHgoGupbI
A1BIfLWTE3ZHj+x8ygGOQcG4uhcsxWJQu+ygKvoDxGkjfQcQ1gEa9KOmWENDUj+Y
8eT+Ghzn+B7Ekl6RjgKGtNJKgVycwPqWJwR0JgWKRVlkeGrAfl6RNerAPwTODHj0
S+vmYeo6ZtrmHluagnP/hk2SB0yr6DmUITdhfE7LpMKAGutQ32PN1n5eDKpGHrez
LWcvJ1beLCEVZk2fJe1r7pqZtMWPIu1wJmla+NyS/7UOuagHcskINm3cSsqNApRn
ujMNALDGYozDhMl5/QJi9TJaqcwB2iRocDppkB75XnpcgLuKIION3KNi3jEhsBF8
DMIIF6/cPPFMSqvsoMcaFgm1CIz+AP5yo0LSfR/Rb2RGh+nhWFmBcNyLO6/bIWih
D2ebrfCfq+SEzS/bqm3zoAXwShw3464xNuU/m9N8eF7erwUY8o2TDn+XwtXaoOlF
kBkaNk8deBwwhIjLG6Pm
=wFNB
-----END PGP SIGNATURE-----

[PATCH 6/8] reg: Add wchar/raw data conversion functions

[Jonathan Vollebregt]

> > +static LSTATUS wchar_get_data(  const WCHAR *input,     const DWORD type,
> > -
> >                            const WCHAR separator,  DWORD *size_out,    BYTE **out)
>
> This whitespace use is a bit weird. Is there a precedent for it?

Nope. I think it's far easier to read with some indentation (eg patch 
7), but if you'd rather have it the old way I can put it back.
> > -
> >                /* If it's the first character or the previous one was a separator */
> > -
> >                if (!p || temp[p - 1] == 0)
> > -
> >                {
> > -
> >                    HeapFree(GetProcessHeap(), 0, temp);
> > -
> >                    return ERROR_BAD_COMMAND;
> > -
> >                }
>
> What does advapi32 do if you have empty substrings?

advapi32 just takes some data and copies it without validation. With 
RegSetValueEx I could give a REG_DWORD value a 1kb string consisting 
entirely of control characters and get it back in one piece.

Input has to be handled by the program, and it's basically up to the 
program to determine the syntax (Regedit for example uses newlines as 
separators) so these are just quirks of reg.exe
> Does native write different error messages for a negative DWORD, improperly separated REG_MULTI_SZ and invalid REG_BINARY?

Assuming you mean error codes: Native advapi32 doesn't validate, and 
native reg only returns 1 or 0.