[PATCH v4 5/8] reg.exe: Add wchar/raw data conversion functions

[Stefan Dösinger]

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

This code could use more tests, especially for REG_SZ_MULTI and REG_BINARY.

Am 2014-10-09 10:55, schrieb Jonathan Vollebregt:
> +static WCHAR *data_get_wchar(const BYTE *data, const DWORD size, const DWORD type)
This function is not used until the next patch. From a design point of view it unfortunate that you allocate a new string for the REG_SZ and REG_SZ_MULTI case and essentially just do a memcpy and free the old string. As long as you just need this function to print the values it would be better to add a function that prints directly.

Keep reg export in mind though. It may need the function like this, or maybe a printing function that is passed a file descriptor.

> +            do
> +            {
> +                lstrcpyW(output+i, &input[i]);
> +
> +                i += strlenW(&input[i]) + 1;
> +
> +                if (input[i] != 0)
> +                    output[i - 1] = ',';
> +            } while (input[i]);
If I understand this correctly this replaces 0 WCHARs with ',' . There should be nicer ways to do this.

> +static BYTE *wchar_get_data(const WCHAR *input,     const DWORD type,
> +                            const WCHAR seperator,  DWORD *size_out)
> ...
> +        case REG_SZ:
> +        case REG_EXPAND_SZ:
> +        {
> +            i = (strlenW(input) + 1) * sizeof(WCHAR);
> +            output = HeapAlloc(GetProcessHeap(), 0, i);
> +            lstrcpyW((WCHAR *) output, input);
> +            *size_out = i;
> +            return output;
> +        }
This also has unfortunate copying behavior. Maybe there is a nicer way to solve this.

> +            for (i = 0, p = 0; i <= strlenW(input); i++, p++)
> +            {
> +                if (input[i] == seperator)
Please watch the spelling of separator. This confused me a bit in my code search :-) .

> +                    temp[p] = 0;
> +                else if (seperator == 0 && input[i] == '\\' && input[i + 1] == '0')
How does a 0 separator work? The command line interpreter would fail to parse such a string. msdn claims that the default separator is '\0', but maybe they mean a backslash ('\\') like your code uses. If the default separator is a backslash it would be nicer to handle this default in the code that parses the command line.

> +                {
> +                    temp[p] = 0;
> +                    i++;
> +                }
I don't understand what's special about a 0 or \ separator as far as this loop is concerned.

> +            if (p % 2)
if (p & 1) is nicer to check for odd / even (i.e., the lowest bit). The compiler should be able to to this optimization as well.

> +            p /= 2;
p >>= 1 . Or you could consider incrementing i by 2 in the for loop below.

> +            for (i = 0; i < p; i++)
> +                temp[i] = (temp[i * 2] << 4) | temp[i * 2 + 1];

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJUOpgoAAoJEN0/YqbEcdMwyLYP+QFSbpZLsy/Au6uKLQNfcRqS
MnHxgrB1wq+RxfDbQ6UdzT6gIVKhEII0c//gZ32H5Fr/+zIofaa38XmtH6Fp8MMg
p1s+ubFeZ29vMaNER28zwK/QM6Allwo+JLjqLSM6W2cgrfe1kU/iPWGC97nJktEj
cG4wsAdcgYCHvkOH/06ZIC/zSbWyG0yZJSgKfLqfgEbuD/TAmmkXh0TVMm9Znp88
J6mLhcYSbfTK2oHzU9nXETTa5oFR/VfHQHZ7asa+q14kftl24Y5JCs1VD6BK/G4p
TjD5jQhIN9L5FHKj79n+9y5eeYjrIiMhdi81ey3Bm8it03ZPUN3aAjXf6CD6UmBu
wxHZwB8L1PALJsDBK+E7a8QgeteWxMjX07m2FdfFNewj4IFq4eoASdeINd8Zd1rt
TpWl2/jpKz4PecRvGeNKOEgqx+mFcSRWiXErytWtEmJzz+aPEjdjFXesf6aWFB/l
+ZZxc202He9JXacPFvmCuL0ltWStJqdSZRZg/FkIgqXZkWfj1bwxDYpu+7Jv6243
KrQrMUGna1VXGM6tAfYph0BLtZnksTwFS6MEoPzirf2Yqe3uuQzecQqhyaStaESJ
5HS+6NTEZT2LhvKy3mui970k2THpmQFb7j4f+xdmKHEj6qdsoH+SHouJgkig/KLk
x28pWN1J80RCE92BQdYV
=tqtd
-----END PGP SIGNATURE-----