Re: reg.exe: Add query function

[Nikolay Sivov]

On 8/1/2013 17:44, Hugh McMaster wrote:
> This patch builds significantly on the query function stub in programs/reg.exe. The patch includes recursion for subkeys.
>
> Some examples of usage:
>
> wine reg.exe query "HKCU"
> wine reg.exe query "HKCU\Console"
> wine reg.exe query "HKCU\Console" /v "ScreenSize"
> wine reg.exe query "HKCU\Software" /s
>
> ---
>   programs/reg/reg.c |  232 +++++++++++++++++++++++++++++++++++++++++++++++++---
>   1 file changed, 221 insertions(+), 11 deletions(-)
>
> +static LPVOID WINAPI mem_alloc(size_t size)
> +{
> +    return HeapAlloc(GetProcessHeap(), 0, size);
> +}
> +
> +static void WINAPI mem_free(LPVOID mem)
> +{
> +    HeapFree(GetProcessHeap(), 0, mem);
> +}
> Why WINAPI? It's better to follow naming from existing module, there's 
> plenty of them with similar helpers.
> +	case REG_SZ:
> +        case REG_MULTI_SZ:
> +        case REG_EXPAND_SZ:
> No tabs please.
> +static void reg_output_subkey(LPWSTR key_name, LPWSTR subkeyName)
> +{
> +    reg_printfW(formatSW, key_name);
> +
> +    if (subkeyName > 0)
> +    {
> +        reg_printfW(formatSW, slashW);
> +        reg_printfW(formatSW, subkeyName);
> +        reg_printfW(formatSW, newlineW);
> +    }
> +}
> Comparison looks a bit odd. Also why do you need to break it down to 
> three printf calls instead of one?
> +    p = strchrW(key_name,'\\');
> +    if (!p)
> +    {
> +        p = 0;
> +    }
> +    else p++;
> I'm not sure what this is supposed to do.
> +        case REG_BINARY:
> +        case REG_NONE:
> +            pValue = value;
> +            for (i=0; i<valueSize; i++, pValue++)
> +                reg_printfW(formatXW, *pValue);
> +            break;
> There's no need for separate variable here nor for incrementing pointer.

reg.exe: Add query function

[Hugh McMaster]

On Thursday, 1 August 2013 11:55 PM, Nikolay Sivov wrote:
> > +    p = strchrW(key_name,'\\');
> > +    if (!p)
> > +    {
> > +        p = 0;
> > +    }
> > +    else p++;
> > I'm not sure what this is supposed to do.

It is equivalent to the following code;

    p = strchrW(key_name, '\\');
    if (p != NULL) p++;

I've now modified the patch to represent this.
> > +        case REG_BINARY:
> > +        case REG_NONE:
> > +            pValue = value;
> > +            for (i=0; i<valueSize; i++, pValue++)
> > +                reg_printfW(formatXW, *pValue);
> > +            break;
> > There's no need for separate variable here nor for incrementing pointer.

The incrementing pointer is needed because the 'value' is a Byte array. But the separate variable is not needed.

reg.exe: Add query function

[Nikolay Sivov]

On 8/2/2013 12:52, Hugh McMaster wrote:
> > > +        case REG_BINARY:
> > > +        case REG_NONE:
> > > +            pValue = value;
> > > +            for (i=0; i<valueSize; i++, pValue++)
> > > +                reg_printfW(formatXW, *pValue);
> > > +            break;
> > > There's no need for separate variable here nor for incrementing pointer.
> > > The incrementing pointer is needed because the 'value' is a Byte array. But the separate variable is not needed.
> > > Yes, so value[i] will do the same.

reg.exe: Add query function

[Hugh McMaster]

On Friday, 2 August 2013 6:58 PM, Nikolay Sivov wrote:
> > > There's no need for separate variable here nor for incrementing pointer.
> > > The incrementing pointer is needed because the 'value' is a Byte array. But the separate variable is not needed.
> > > Yes, so value[i] will do the same.

Yes, you're quite correct.