Re: msi: Fix use of uninitialized variable (Coverity) (Try 2)

[James Hawkins]

On 6/23/07, Andrew Talbot  wrote:
> This patch should fix Coverity bug CID-562.
> -- Andy.
>
> Changelog:
>   msi: Fix use of uninitialized variable (Coverity).
>
> diff -urN a/dlls/msi/action.c b/dlls/msi/action.c
> --- a/dlls/msi/action.c 2007-06-18 17:52:27.000000000 +0100
> +++ b/dlls/msi/action.c 2007-06-23 17:08:55.000000000 +0100
> @@ -4648,7 +4648,7 @@
>      LPWSTR deformatted, ptr;
>      DWORD flags, type, size;
>      LONG res;
> - HKEY env, root = HKEY_CURRENT_USER;
> -
>
> HKEY env = NULL, root = HKEY_CURRENT_USER;
>
> static const WCHAR environment[] =
>     {'S','y','s','t','e','m','\',
>
> @@ -4759,7 +4759,7 @@
>      res = RegSetValueExW(env, name, 0, type, (LPVOID)newval, size);
>
> done:
> - RegCloseKey(env);
> - if (env) RegCloseKey(env);
> msi_free(deformatted);
> msi_free(data);
> msi_free(newval);

Please don't check env for NULL; RegCloseKey will just fail harmlessly
if env is NULL.  We spent a lot of time removing such checks.

msi: Fix use of uninitialized variable (Coverity) (Try 2)

[Andrew Talbot]

James Hawkins wrote:
> Please don't check env for NULL; RegCloseKey will just fail harmlessly
> if env is NULL.  We spent a lot of time removing such checks.

OK, I shall re-post my first effort as "Try 3".

msi: Fix use of uninitialized variable (Coverity) (Try 2)

[Paul Vriens]

Andrew Talbot wrote:
> James Hawkins wrote:
> > Please don't check env for NULL; RegCloseKey will just fail harmlessly
> > if env is NULL.  We spent a lot of time removing such checks.
>
> OK, I shall re-post my first effort as "Try 3".

But then why don't you just set env to NULL and leave the rest as is. The only 
thing that will change is the lasterror probably if you don't check for NULL.

Cheers,

Paul.

msi: Fix use of uninitialized variable (Coverity) (Try 2)

[Tom Spear]

On 6/23/07, James Hawkins  wrote:
> On 6/23/07, Andrew Talbot  wrote:
> > This patch should fix Coverity bug CID-562.
> > -- Andy.
> >
> > Changelog:
> >   msi: Fix use of uninitialized variable (Coverity).
> >
> > diff -urN a/dlls/msi/action.c b/dlls/msi/action.c
> > --- a/dlls/msi/action.c 2007-06-18 17:52:27.000000000 +0100
> > +++ b/dlls/msi/action.c 2007-06-23 17:08:55.000000000 +0100
> > @@ -4648,7 +4648,7 @@
> >      LPWSTR deformatted, ptr;
> >      DWORD flags, type, size;
> >      LONG res;
> > - HKEY env, root = HKEY_CURRENT_USER;
> > -
> >
> > HKEY env = NULL, root = HKEY_CURRENT_USER;
> >
> > static const WCHAR environment[] =
> >     {'S','y','s','t','e','m','\',
> >
> > @@ -4759,7 +4759,7 @@
> >      res = RegSetValueExW(env, name, 0, type, (LPVOID)newval, size);
> >
> > done:
> > - RegCloseKey(env);
> > - if (env) RegCloseKey(env);
> > msi_free(deformatted);
> > msi_free(data);
> > msi_free(newval);
>
> Please don't check env for NULL; RegCloseKey will just fail harmlessly
> if env is NULL.  We spent a lot of time removing such checks.

Perhaps a conformance test is in order, because from what I have read,
RegCloseKey returns a nonzero error code on any error, including a
NULL variable being passed to it, which screws up obtaining the
original error if x program or api tries to use GetLastError.  We
don't want bugs in wine, unless it is duplicating a windows bug, and
what I see if we don't check the way Andrew has, is a bug that I am
pretty sure does not exist in windows.

RegCloseKey documentation (our RegCloseKey function matches this):
http://msdn2.microsoft.com/en-us/library/ms724837.aspx