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 :
> programs/reg/reg.c | 166 +++++++++++++++++++++++++++--------------------------
> 1 file changed, 86 insertions(+), 80 deletions(-)
>
> <0001-reg-Add-path-key-conversion-functions.patch>