Re: [PATCH 1/4] reg: Add path/key conversion functions

[Stefan Dösinger]

I think these patches can be split further. And yeah, I could have thought of some of this sooner :-( .

1.1: A patch that replaces the if chains with the table
1.2: A patch that merges the duplicated string splitting with reg_open

2.1: Implement REG_MULTI_SZ. Use this patch to strip LPCRAP from the function definition
2.2: Improve REG_DWORD. I think you can merge the REG_DWORD_BIG_ENDIAN support into this patch, since it is just another REG_DWORD.
2.3: Add REG_NONE. Just a one-liner and todo_wine removal.
2.4: Add REG_EXPAND_SZ. Another one-liner.
2.5: Add REG_BINARY.

Patch 3:
I think at least the check
> +    if (value_name && value_empty)
> +    {
> +        reg_message(STRING_INVALID_CMDLINE);
> +        return 1;
> +    }
> Is independent of the rest. I may be missing something though.

Another question that pops up in my mind is this:
> +        if (value_name && !value_name[0])
> +            value_name = NULL;
> This is interesting together with the value_name && value_empty check. I suspect /v "" /ve still results in an error, but it is worth a test in a separate patch. This can be after the existing series as far as I'm concerned.
> +                if (!argvW[++i][0] || argvW[i][1])
> +                {
> +                    reg_message(STRING_INVALID_CMDLINE);
> +                    return 1;
> +                }
> Can also go in a separate patch.
> +        if (err != ERROR_SUCCESS){
> I expect Alexandre to hit me with a brick now, but this is inconsistent with the rest.

Patch 4 can be split in a similar fashion:
> +    /* Mutually exclusive options */
> +    if ((!!value_name + !!value_empty + !!value_all) > 1)
> I think merging the existing (and broken) checks for that is separate from the rest.

The changes you make to handle value_all can also be separated I think. That will make this patch easier to read.

Stefan

Am 04.03.2015 um 17:29 schrieb Jonathan Vollebregt <jnvsor(a)gmail.com>:
> ---
> programs/reg/reg.c | 166 +++++++++++++++++++++++++++--------------------------
> 1 file changed, 86 insertions(+), 80 deletions(-)
>
> <0001-reg-Add-path-key-conversion-functions.patch>