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

[Jasper St. Pierre]

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.


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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.x.org/archives/xorg-devel/attachments/20140709/52485872/attachment-0001.html>