Re: msi: Do not use type value if call to RegQueryValueEx failed.

[James Hawkins]

On 6/12/07, Misha Koshelev <mk144210(a)bcm.edu> wrote:
> This is necessary (bug not sufficient, see my comment in the bug report that I am about
> to post) to fix bug 8678.
>
> It is also clearly correct, as checking the type "returned" by RegQueryValueEx when it fails
> makes no sense.

size = 0;

     res = RegQueryValueExW(env, var, NULL, &type, NULL, &size);

-    if ((res != ERROR_MORE_DATA && res != ERROR_FILE_NOT_FOUND) ||
type != REG_SZ)

+    if ((res != ERROR_MORE_DATA && res != ERROR_FILE_NOT_FOUND) ||

+        (res == ERROR_SUCCESS && type != REG_SZ))

     {

         RegCloseKey(env);

         return res;

This is wrong.  If res == ERROR_SUCCESS, then the first condition
evaluates to TRUE and we fall through to exit, but then again, res
will never be ERROR_SUCCESS after that particular call.  Either way, I
have a patch ready to send in that fixes the entire action.

-- 
James Hawkins

msi: Do not use type value if call to RegQueryValueEx failed.

[Koshelev, Misha Vladislavo]

http://www.winehq.org/pipermail/wine-patches/2007-June/040336.html
 
The third version I sent out (yes I know, I was quite sleepy) _is_ correct though.
 
And that particular call _will_ return ERROR_SUCCESS and _not_ ERROR_MORE_DATA,
see:
http://msdn2.microsoft.com/en-us/library/ms724911.aspx

Specifically, a call to RegQueryValueEx with a NULL data pointer and a valid size pointer returns
ERROR_SUCCESS if the size pointer is filled, _not_ ERROR_MORE_DATA, which is returned
if a valid data pointer is passed.

Misha

________________________________

From: James Hawkins [mailto:truiken(a)gmail.com]
Sent: Wed 6/13/2007 1:22 AM
To: wine-devel(a)winehq.org
Cc: Koshelev, Misha Vladislavo
Subject: Re: msi: Do not use type value if call to RegQueryValueEx failed.



On 6/12/07, Misha Koshelev <mk144210(a)bcm.edu> wrote:
> This is necessary (bug not sufficient, see my comment in the bug report that I am about
> to post) to fix bug 8678.
>
> It is also clearly correct, as checking the type "returned" by RegQueryValueEx when it fails
> makes no sense.

size = 0;

     res = RegQueryValueExW(env, var, NULL, &type, NULL, &size);

-    if ((res != ERROR_MORE_DATA && res != ERROR_FILE_NOT_FOUND) ||
type != REG_SZ)

+    if ((res != ERROR_MORE_DATA && res != ERROR_FILE_NOT_FOUND) ||

+        (res == ERROR_SUCCESS && type != REG_SZ))

     {

         RegCloseKey(env);

         return res;

This is wrong.  If res == ERROR_SUCCESS, then the first condition
evaluates to TRUE and we fall through to exit, but then again, res
will never be ERROR_SUCCESS after that particular call.  Either way, I
have a patch ready to send in that fixes the entire action.

--
James Hawkins

msi: Do not use type value if call to RegQueryValueEx failed.

[Misha Koshelev]

On Tue, 2007-06-12 at 23:22 -0700, James Hawkins wrote:
> On 6/12/07, Misha Koshelev <mk144210(a)bcm.edu> wrote:
> > This is necessary (bug not sufficient, see my comment in the bug report that I am about
> > to post) to fix bug 8678.
> >
> > It is also clearly correct, as checking the type "returned" by RegQueryValueEx when it fails
> > makes no sense.
>
> size = 0;
>
> res = RegQueryValueExW(env, var, NULL, &type, NULL, &size);
>
> -    if ((res != ERROR_MORE_DATA && res != ERROR_FILE_NOT_FOUND) ||
> type != REG_SZ)
>
> +    if ((res != ERROR_MORE_DATA && res != ERROR_FILE_NOT_FOUND) ||
>
> +        (res == ERROR_SUCCESS && type != REG_SZ))
>
> {
>
> RegCloseKey(env);
>
> return res;
>
> This is wrong.  If res == ERROR_SUCCESS, then the first condition
> evaluates to TRUE and we fall through to exit, but then again, res
> will never be ERROR_SUCCESS after that particular call.  Either way, I
> have a patch ready to send in that fixes the entire action.

---
http://msdn2.microsoft.com/en-us/library/ms724911.aspx
If lpData is NULL, and lpcbData is non-NULL, the function returns
ERROR_SUCCESS and stores the size of the data, in bytes, in the variable
pointed to by lpcbData. This enables an application to determine the
best way to allocate a buffer for the value's data.
---

Clearly a rewrite is better than changing one line, but I hope your
rewrite checks for ERROR_SUCCESS and not ERROR_MORE_DATA in this case.

Misha