Re: [resend PATCH] ntdll: Tolerate null handle in DeregisterWait

[Bruno Jesus]

On Fri, Aug 19, 2016 at 6:24 PM, Keno Fischer  wrote:

Hi, is the subject supposed to be Tolerate NULL handle in UnregisterWait?
> diff --git a/dlls/kernel32/tests/thread.c b/dlls/kernel32/tests/thread.c
> index a9a04df..44c6278 100644
> --- a/dlls/kernel32/tests/thread.c
> +++ b/dlls/kernel32/tests/thread.c
> @@ -1245,6 +1245,11 @@ static void test_RegisterWaitForSingleObject(void)
>  ret = pUnregisterWait(wait_handle);
>  ok(ret, "UnregisterWait failed with error %d\n", GetLastError());
> -
> - /* Test UnregisterWait null argument case */
> - ret = pUnregisterWait(0);
> - ok(!ret && GetLastError() == ERROR_INVALID_HANDLE,
> -
>    "UnregisterWait failed with unexpected error %d\n", GetLastError());
>
> }

Please test ret and GetLastError separately as the rest of the code
does most times.

If you see other tests that test GetLastError you will notice that
they call SetLastError before calling the tested function to ensure
the error is not from a previous test. So add a
SetLastError(0xdeadbeef) before the call to pUnregisterWait to ensure
the ERROR_INVALID_HANDLE returned is from UnregisterWait.

Also for consistency why not pUnregisterWait(NULL) instead of (0)?

[resend PATCH] ntdll: Tolerate null handle in DeregisterWait

[Keno Fischer]

Thanks for committing this. I was going to address the review
comments, but didn't get to that before this was committed. Would you
like me to submit a separate patch to address the review comments?

On Fri, Aug 19, 2016 at 5:57 PM, Bruno Jesus  wrote:
> On Fri, Aug 19, 2016 at 6:24 PM, Keno Fischer  wrote:
>
> Hi, is the subject supposed to be Tolerate NULL handle in UnregisterWait?
> > diff --git a/dlls/kernel32/tests/thread.c b/dlls/kernel32/tests/thread.c
> > index a9a04df..44c6278 100644
> > --- a/dlls/kernel32/tests/thread.c
> > +++ b/dlls/kernel32/tests/thread.c
> > @@ -1245,6 +1245,11 @@ static void test_RegisterWaitForSingleObject(void)
> >  ret = pUnregisterWait(wait_handle);
> >  ok(ret, "UnregisterWait failed with error %d\n", GetLastError());
> > -
> > - /* Test UnregisterWait null argument case */
> > - ret = pUnregisterWait(0);
> > - ok(!ret && GetLastError() == ERROR_INVALID_HANDLE,
> > -
> >    "UnregisterWait failed with unexpected error %d\n", GetLastError());
> >
> > }
>
> Please test ret and GetLastError separately as the rest of the code
> does most times.
>
> If you see other tests that test GetLastError you will notice that
> they call SetLastError before calling the tested function to ensure
> the error is not from a previous test. So add a
> SetLastError(0xdeadbeef) before the call to pUnregisterWait to ensure
> the ERROR_INVALID_HANDLE returned is from UnregisterWait.
>
> Also for consistency why not pUnregisterWait(NULL) instead of (0)?

[resend PATCH] ntdll: Tolerate null handle in DeregisterWait

[Alexandre Julliard]

Keno Fischer  writes:
> Thanks for committing this. I was going to address the review
> comments, but didn't get to that before this was committed. Would you
> like me to submit a separate patch to address the review comments?

Sure, sorry that I jumped the gun.