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

2007-06-24 Thread 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.

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

2007-06-23 Thread James Hawkins 

On 6/23/07, Andrew Talbot [EMAIL PROTECTED] 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.0 +0100
+++ b/dlls/msi/action.c 2007-06-23 17:08:55.0 +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.

--
James Hawkins

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

2007-06-23 Thread 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.

-- 
Andy.

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

2007-06-23 Thread Tom Spear 

On 6/23/07, James Hawkins [EMAIL PROTECTED] wrote:

On 6/23/07, Andrew Talbot [EMAIL PROTECTED] 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.0 +0100
 +++ b/dlls/msi/action.c 2007-06-23 17:08:55.0 +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


--
Thanks

Tom