Re: RegOpenKeyExW() Question

2007-06-23 Thread Andrew Talbot
Tom Spear wrote:

 At the risk of looking like an idiot, I'll take a stab at this..  If
 you RegCloseKey on env, and it is not initialized, then RegCloseKey
 will generate ERROR_INVALID_HANDLE, so I personally think that
 something like the first approach would be the better one to go with.
 env should only be initialized (according to what I have seen) if
 RegOpenKeyExW returns ERROR_SUCCESS.  But I haven't seen every case,
 so there may be something that causes it to be initialized when the
 return is not ERROR_SUCCESS.
 
 
Thanks, Tom. I think I might submit a patch to fix the violation that
Coverity flags, which is prior to the RegOpenKeyW() call, and just insert a
FIXME: comment where the return value of RegOpenKeyW() is tested, without
changing the existing functionality.

-- 
Andy.






RegOpenKeyExW() Question

2007-06-22 Thread Andrew Talbot
Hi,

I want to patch msi:action.c:ITERATE_WriteEnvironmentString() so that it
only calls RegCloseKey(env), in the cleanup, if env has been initialized
(to fix Coverity report CID-562). I can bypass the call to RegCloseKey()
for any early exit that occurs prior to calling RegOpenKeyExW(), and I can
include the call to RegCloseKey() for exits that occur after a successful
call to RegOpenKeyExW(). However, the problem, for me, is what to do if the
call to RegOpenKeyExW() fails: does the occurrence of this initialization
depend on the reason for failure. In other words, do I need something like
the following code to cater for the reason failure occurred?

res = RegOpenKeyExW(root, environment, 0, KEY_ALL_ACCESS, env);

if (res != ERROR_SUCCESS)
if (res == ERROR_INVALID_HANDLE ||
res == RtlNtStatusToDosError(STATUS_BUFFER_OVERFLOW ||
res == RtlNtStatusToDosError(STATUS_INVALID_PARAMETER))
goto done2;
else
goto done1;

...

done1:
RegCloseKey(env);
done2:
...

Or will the following code suffice?

res = RegOpenKeyExW(root, environment, 0, KEY_ALL_ACCESS, env);

if (res != ERROR_SUCCESS)
goto done2;

...

done1:
RegCloseKey(env);
done2:
...

Thanks,

-- 
Andy.






Re: RegOpenKeyExW() Question

2007-06-22 Thread Tom Spear

On 6/22/07, Andrew Talbot [EMAIL PROTECTED] wrote:

Hi,

I want to patch msi:action.c:ITERATE_WriteEnvironmentString() so that it
only calls RegCloseKey(env), in the cleanup, if env has been initialized
(to fix Coverity report CID-562). I can bypass the call to RegCloseKey()
for any early exit that occurs prior to calling RegOpenKeyExW(), and I can
include the call to RegCloseKey() for exits that occur after a successful
call to RegOpenKeyExW(). However, the problem, for me, is what to do if the
call to RegOpenKeyExW() fails: does the occurrence of this initialization
depend on the reason for failure. In other words, do I need something like
the following code to cater for the reason failure occurred?

   res = RegOpenKeyExW(root, environment, 0, KEY_ALL_ACCESS, env);

   if (res != ERROR_SUCCESS)
   if (res == ERROR_INVALID_HANDLE ||
   res == RtlNtStatusToDosError(STATUS_BUFFER_OVERFLOW ||
   res == RtlNtStatusToDosError(STATUS_INVALID_PARAMETER))
   goto done2;
   else
   goto done1;

   ...

done1:
   RegCloseKey(env);
done2:
   ...

Or will the following code suffice?

   res = RegOpenKeyExW(root, environment, 0, KEY_ALL_ACCESS, env);

   if (res != ERROR_SUCCESS)
   goto done2;

   ...

done1:
   RegCloseKey(env);
done2:
   ...


At the risk of looking like an idiot, I'll take a stab at this..  If
you RegCloseKey on env, and it is not initialized, then RegCloseKey
will generate ERROR_INVALID_HANDLE, so I personally think that
something like the first approach would be the better one to go with.
env should only be initialized (according to what I have seen) if
RegOpenKeyExW returns ERROR_SUCCESS.  But I haven't seen every case,
so there may be something that causes it to be initialized when the
return is not ERROR_SUCCESS.


--
Thanks

Tom