Re: [PATCH 3/7] reg: Fail if the data for REG_DWORD cannot be converted into a DWORD

[Alexandre Julliard]

Hugh McMaster  writes:
> @@ -376,6 +376,12 @@ static int reg_add(WCHAR *key_name, WCHAR *value_name, BOOL value_empty,
>          if (data)
>              reg_data = get_regdata(data,reg_type,separator,&reg_count);
> -
>    if (!reg_data && reg_type == REG_DWORD)
> -
>    {
> -
>        RegCloseKey(subkey);
> -
>        return 1;
> -
>    }

Shouldn't this fail for other types too?

[PATCH 3/7] reg: Fail if the data for REG_DWORD cannot be converted into a DWORD

[Hugh McMaster]

On Mon, 15 Feb 2016 13:59:04 +0900, Alexandre Julliard wrote:
> Hugh McMaster writes:
> > @@ -376,6 +376,12 @@ static int reg_add(WCHAR *key_name, WCHAR *value_name, BOOL value_empty,
> >          if (data)
> >              reg_data = get_regdata(data,reg_type,separator,&reg_count);
> > -
> >    if (!reg_data && reg_type == REG_DWORD)
> > -
> >    {
> > -
> >        RegCloseKey(subkey);
> > -
> >        return 1;
> > -
> >    }
>
> Shouldn't this fail for other types too?

Probably -- but I haven't looked at them yet.

REG_DWORD_LITTLE_ENDIAN and REG_BINARY would most likely cause a failure.
If they do, I'll use a switch with fall-through cases to handle the clean-up and exit.

The common registry string types -- REG_SZ, REG_MULTI_SZ and REG_EXPAND_SZ --
don't fail in this way. Based on the tests we have, the Windows version just copies
the string data to the registry as appropriate.

--
Hugh McMaster

[PATCH 3/7] reg: Fail if the data for REG_DWORD cannot be converted into a DWORD

[Alexandre Julliard]

Hugh McMaster  writes:
> On Mon, 15 Feb 2016 13:59:04 +0900, Alexandre Julliard wrote:
> > Hugh McMaster writes:
> > > @@ -376,6 +376,12 @@ static int reg_add(WCHAR *key_name, WCHAR *value_name, BOOL value_empty,
> > >          if (data)
> > >              reg_data = get_regdata(data,reg_type,separator,&reg_count);
> > > -
> > >    if (!reg_data && reg_type == REG_DWORD)
> > > -
> > >    {
> > > -
> > >        RegCloseKey(subkey);
> > > -
> > >        return 1;
> > > -
> > >    }
> >
> > Shouldn't this fail for other types too?
>
> Probably -- but I haven't looked at them yet.
>
> REG_DWORD_LITTLE_ENDIAN and REG_BINARY would most likely cause a failure.
> If they do, I'll use a switch with fall-through cases to handle the clean-up and exit.
>
> The common registry string types -- REG_SZ, REG_MULTI_SZ and REG_EXPAND_SZ --
> don't fail in this way. Based on the tests we have, the Windows version just copies
> the string data to the registry as appropriate.

Yes, but then get_regdata is not going to fail, it will simply copy the
string. My point is that if get_regdata fails to make sense of the data,
there's nothing to set so we should abort, for any key type. Not to
mention that it already prints an error, so setting the key anyway looks
very wrong.