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

[Christian Costa]

2012/10/24 Nikolay Sivov
> On 10/24/2012 11:33, Christian Costa wrote:
>
> 2012/10/24 Dmitry Timoshkov
> > Christian Costa  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.