[PATCH] user32: Fix error handling in MapWindowPoints, ClientToScreen and ScreenToClient and add tests for them. (try 3)

Christian Costa titan.costa at gmail.com
Wed Oct 24 06:02:07 CDT 2012


2012/10/24 Nikolay Sivov <bunglehead at gmail.com>

>  On 10/24/2012 11:33, Christian Costa wrote:
>
>
>
> 2012/10/24 Dmitry Timoshkov <dmitry at baikal.ru>
>
>> Christian Costa <titan.costa at gmail.com> wrote:
>>
>> >  BOOL WINAPI ClientToScreen( HWND hwnd, LPPOINT lppnt )
>> >  {
>> > +    DWORD error = GetLastError();
>> > +
>> > +    if (!hwnd)
>> > +    {
>> > +        SetLastError( ERROR_INVALID_WINDOW_HANDLE );
>> > +        return FALSE;
>> > +    }
>> > +
>> > +    SetLastError( 0xdeadbeef );
>> >      MapWindowPoints( hwnd, 0, lppnt, 1 );
>> > +
>> > +    if (GetLastError() != 0xdeadbeef)
>> > +        return FALSE;
>> > +
>> > +    SetLastError(error);
>> >      return TRUE;
>> >  }
>>
>>  As been said before these games with saving/restoring last error value
>> are broken.
>>
>>
>>  Last time you said wrong so what do you mean by wrong or broken?
> The only way to know if MapWindowPoints fails is to set last error first
> and check it
> after. If I don't restore the previous value, the tests will not pass. Of
> course I can
> arrange the tests but hey. And I can check windows handle here as
> Alexandre said.
>
> If error reporting from MapWindowPoints is not sufficient to use it
> somewhere else you can always move its guts to
> a separate function and report failure as you want. By the way, it only
> fails on invalid window handles I guess, is that right?
> If you need this behaviour you have a failing WIN_GetPtr() in this case.
> Messing with last error is not a solution obviously,
> and I don't see where MapWindowPoints even sets it.
>
>
> Sounds good. I'll do that. Basically WINPOS_GetWinOffset get most part the
job in our case and I already modified it to return an error.
Yes it fails only for invalid handles. WIN_GetPtr is not enough because it
just return WND_OTHER_PROCESS to call the server to ask
if it exists on another process. It's what IsWindow does
(and WINPOS_GetWinOffset).
MapWindowPoints can fail (see test). MSDN says to call SetLastError first
before calling it and read the value.
I will send a patch using WINPOS_GetWinOffset. Thanks.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20121024/63e46548/attachment.html>


More information about the wine-devel mailing list