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