[PATCH libXi 2/2] XIPassiveGrab: Fix completely broken locking in XIGrabTouchBegin

[Peter Hutterer]

On Wed, Jul 09, 2014 at 06:45:06AM -0400, Jasper St. Pierre wrote:
> But all the other existing code that uses _XiCheckExtInit expects it to
> unlock on error, but not on success. Peter wrote this code, though, so I'll
> let him have the final say.

yeah, if someone wants to fix this on a global scale  (requires XI 1.x code
as well iirc) be my guest, but for now we just go with what we have.

both pushed, thanks.

Cheers,
   Peter

> 
> 
> On Wed, Jul 9, 2014 at 6:31 AM, walter harms <wharms at bfs.de> wrote:
> 
> >
> >
> > Am 09.07.2014 12:23, schrieb Jasper St. Pierre:
> > > _XiCheckExtInit already unlocks the display on error. Yes, it's terrible.
> > >
> > >
> >
> > In this case,
> > it would be better to move the lock/unlock into _XiCheckExtInit.
> >
> > re,
> >  wh
> >
> >
> > > On Wed, Jul 9, 2014 at 3:23 AM, walter harms <wharms at bfs.de> wrote:
> > >
> > >>
> > >>
> > >> Am 08.07.2014 23:01, schrieb Jasper St. Pierre:
> > >>> _XIPassiveGrabDevice calls LockDisplay as the first thing it does. That
> > >>> means that it expects the display to be unlocked. XIGrabTouchBegin
> > locks
> > >>> the display to check for the XI extension, and then never unlocks it.
> > >>> Effectively, this meant that anybody that called XIGrabTouchBegin after
> > >>> XInitThreads just got a deadlock.
> > >>>
> > >>> Cool.
> > >>> ---
> > >>>  src/XIPassiveGrab.c | 1 +
> > >>>  1 file changed, 1 insertion(+)
> > >>>
> > >>> diff --git a/src/XIPassiveGrab.c b/src/XIPassiveGrab.c
> > >>> index f3a9924..88f1aff 100644
> > >>> --- a/src/XIPassiveGrab.c
> > >>> +++ b/src/XIPassiveGrab.c
> > >>> @@ -166,6 +166,7 @@ XIGrabTouchBegin(Display *dpy, int deviceid, Window
> > >> grab_window,
> > >>>      LockDisplay(dpy);
> > >>>      if (_XiCheckExtInit(dpy, XInput_2_2, extinfo) == -1)
> > >>>       return -1;
> > >>> +    UnlockDisplay(dpy);
> > >>>
> > >>>      /* FIXME: allow selection of GrabMode for paired devices? */
> > >>>      return _XIPassiveGrabDevice(dpy, deviceid, XIGrabtypeTouchBegin,
> > 0,
> > >>
> > >>
> > >> I am not an expert on this but you should  unlock the display on error
> > >> also.
> > >> I would do it this way:
> > >>
> > >> LockDisplay(dpy);
> > >> err=_XiCheckExtInit(dpy, XInput_2_2, extinfo);
> > >> UnlockDisplay(dpy);
> > >> if (err == -1)
> > >>         return -1;
> > >>
> > >> jz'ust my 2 cents
> > >> re,
> > >>  wh
> > >>
> > >> _______________________________________________
> > >> xorg-devel at lists.x.org: X.Org development
> > >> Archives: http://lists.x.org/archives/xorg-devel
> > >> Info: http://lists.x.org/mailman/listinfo/xorg-devel
> > >>
> > >
> > >
> > >
> >
> 
> 
> 
> -- 
>   Jasper

> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel