[PATCH (v5) xserver 2/8] Input: Make CheckPassiveGrabsOnWindow take InternalEvent

Peter Hutterer peter.hutterer at who-t.net
Thu Jan 20 15:24:04 PST 2011


On Wed, Jan 19, 2011 at 11:11:44PM +0000, Daniel Stone wrote:
> Previously, it only took DeviceEvents, but it would be much more useful
> if it took InternalEvents.  Any event that activates a grab must still
> be a DeviceEvent, so put in a check to enforce this.  Also fix a tiny
> (but harmless) buglet where we would allocate too much space for the
> frozen event.
> 
> Change all callers to make the appropriate casts.
> 
> Signed-off-by: Daniel Stone <daniel at fooishbar.org>
> ---
> 
> v5: New.
> 
>  dix/events.c  |   76 +++++++++++++++++++++++++++++++++++++++------------------
>  include/dix.h |    2 +-
>  2 files changed, 53 insertions(+), 25 deletions(-)
> 
> diff --git a/dix/events.c b/dix/events.c
> index 5f8ce39..dfe938b 100644
> --- a/dix/events.c
> +++ b/dix/events.c
> @@ -2629,7 +2629,8 @@ ActivateFocusInGrab(DeviceIntPtr dev, WindowPtr old, WindowPtr win)
>      event.deviceid = dev->id;
>      event.sourceid = dev->id;
>      event.detail.button = 0;
> -    rc = (CheckPassiveGrabsOnWindow(win, dev, &event, FALSE, TRUE) != NULL);
> +    rc = (CheckPassiveGrabsOnWindow(win, dev, (InternalEvent *) &event, FALSE,
> +                                    TRUE) != NULL);
>      if (rc)
>          DoEnterLeaveEvents(dev, dev->id, old, win, XINotifyPassiveUngrab);
>      return rc;
> @@ -2666,7 +2667,8 @@ ActivateEnterGrab(DeviceIntPtr dev, WindowPtr old, WindowPtr win)
>      event.deviceid = dev->id;
>      event.sourceid = dev->id;
>      event.detail.button = 0;
> -    rc = (CheckPassiveGrabsOnWindow(win, dev, &event, FALSE, TRUE) != NULL);
> +    rc = (CheckPassiveGrabsOnWindow(win, dev, (InternalEvent *) &event, FALSE,
> +                                    TRUE) != NULL);
>      if (rc)
>          DoEnterLeaveEvents(dev, dev->id, old, win, XINotifyPassiveGrab);
>      return rc;
> @@ -3353,7 +3355,7 @@ GrabPtr
>  CheckPassiveGrabsOnWindow(
>      WindowPtr pWin,
>      DeviceIntPtr device,
> -    DeviceEvent *event,
> +    InternalEvent *event,
>      BOOL checkCore,
>      BOOL activate)
>  {
> @@ -3370,9 +3372,22 @@ CheckPassiveGrabsOnWindow(
>  	return NULL;
>      /* Fill out the grab details, but leave the type for later before
>       * comparing */
> +    switch (event->any.type)
> +    {
> +    case ET_KeyPress:
> +    case ET_KeyRelease:

common style is to indent case

> +        tempGrab.detail.exact = event->device_event.detail.key;
> +        break;
> +    case ET_ButtonPress:
> +    case ET_ButtonRelease:
> +        tempGrab.detail.exact = event->device_event.detail.button;
> +        break;
> +    default:
> +        tempGrab.detail.exact = 0;
> +        break;
> +    }
>      tempGrab.window = pWin;
>      tempGrab.device = device;
> -    tempGrab.detail.exact = event->detail.key;
>      tempGrab.detail.pMask = NULL;
>      tempGrab.modifiersDetail.pMask = NULL;
>      tempGrab.next = NULL;
> @@ -3380,6 +3395,9 @@ CheckPassiveGrabsOnWindow(
>      {
>  	DeviceIntPtr	gdev;
>  	XkbSrvInfoPtr	xkbi = NULL;
> +	int rc, count = 0;
> +	xEvent *xE = NULL;
> +	xEvent core;
>  
>  	gdev= grab->modifierDevice;
>          if (grab->grabtype == GRABTYPE_CORE)
> @@ -3405,16 +3423,15 @@ CheckPassiveGrabsOnWindow(
>          tempGrab.modifiersDetail.exact = xkbi ? xkbi->state.grab_mods : 0;
>  
>          /* Check for XI2 and XI grabs first */
> -        tempGrab.type = GetXI2Type((InternalEvent*)event);
> +        tempGrab.type = GetXI2Type(event);
>          tempGrab.grabtype = GRABTYPE_XI2;
>          if (GrabMatchesSecond(&tempGrab, grab, FALSE))
>              match = XI2_MATCH;
>  
> -        tempGrab.detail.exact = event->detail.key;
>          if (!match)
>          {
>              tempGrab.grabtype = GRABTYPE_XI;
> -            if ((tempGrab.type = GetXIType((InternalEvent*)event)) &&
> +            if ((tempGrab.type = GetXIType(event)) &&
>                  (GrabMatchesSecond(&tempGrab, grab, FALSE)))
>                  match = XI_MATCH;
>          }
> @@ -3423,7 +3440,7 @@ CheckPassiveGrabsOnWindow(
>          if (!match && checkCore)
>          {
>              tempGrab.grabtype = GRABTYPE_CORE;
> -            if ((tempGrab.type = GetCoreType((InternalEvent*)event)) &&
> +            if ((tempGrab.type = GetCoreType(event)) &&
>                  (GrabMatchesSecond(&tempGrab, grab, TRUE)))
>                  match = CORE_MATCH;
>          }
> @@ -3432,12 +3449,6 @@ CheckPassiveGrabsOnWindow(
>  	     (grab->confineTo->realized &&
>  				BorderSizeNotEmpty(device, grab->confineTo))))
>  	{
> -            int rc, count = 0;
> -            xEvent *xE = NULL;
> -            xEvent core;
> -
> -            event->corestate &= 0x1f00;
> -            event->corestate |= tempGrab.modifiersDetail.exact & (~0x1f00);
>              grabinfo = &device->deviceGrab;
>              /* In some cases a passive core grab may exist, but the client
>               * already has a core grab on some other device. In this case we
> @@ -3483,38 +3494,53 @@ CheckPassiveGrabsOnWindow(
>  
>              if (!activate)
>                  return grab;
> +            else if (!GetXIType(event) && !GetCoreType(event))
> +                FatalError("Event type %d in CheckPassiveGrabsOnWindow is"
> +                           " neither XI 1.x nor core\n", event->any.type);

no. return gracefully and spam the logs. bugs like this are usually easy to
find once they can be reproduced and in the meantime the server will
continue working for those affected withouth intentionally quitting whenever
they hit a wrong key.

> +
> +            /* The only consumers of corestate are Xi 1.x and core events,
> +             * which are guaranteed to come from DeviceEvents. */
> +            if (match & (XI_MATCH | CORE_MATCH))
> +            {
> +                event->device_event.corestate &= 0x1f00;
> +                event->device_event.corestate |=
> +                    tempGrab.modifiersDetail.exact & (~0x1f00);
> +            }
>  
>              if (match & CORE_MATCH)
>              {
> -                rc = EventToCore((InternalEvent*)event, &core);
> +                rc = EventToCore(event, &core);
>                  if (rc != Success)
>                  {
>                      if (rc != BadMatch)
>                          ErrorF("[dix] %s: core conversion failed in CPGFW "
> -                                "(%d, %d).\n", device->name, event->type, rc);
> +                                "(%d, %d).\n", device->name, event->any.type,
> +                                rc);
>                      continue;
>                  }
>                  xE = &core;
>                  count = 1;
>              } else if (match & XI2_MATCH)
>              {
> -                rc = EventToXI2((InternalEvent*)event, &xE);
> +                rc = EventToXI2(event, &xE);
>                  if (rc != Success)
>                  {
>                      if (rc != BadMatch)
>                          ErrorF("[dix] %s: XI2 conversion failed in CPGFW "
> -                                "(%d, %d).\n", device->name, event->type, rc);
> +                                "(%d, %d).\n", device->name, event->any.type,
> +                                rc);
>                      continue;
>                  }
>                  count = 1;
>              } else
>              {
> -                rc = EventToXI((InternalEvent*)event, &xE, &count);
> +                rc = EventToXI(event, &xE, &count);
>                  if (rc != Success)
>                  {
>                      if (rc != BadMatch)
>                          ErrorF("[dix] %s: XI conversion failed in CPGFW "
> -                                "(%d, %d).\n", device->name, event->type, rc);
> +                                "(%d, %d).\n", device->name, event->any.type,
> +                                rc);
>                      continue;
>                  }
>              }
> @@ -3533,8 +3559,8 @@ CheckPassiveGrabsOnWindow(
>  	    if (grabinfo->sync.state == FROZEN_NO_EVENT)
>  	    {
>                  if (!grabinfo->sync.event)
> -                    grabinfo->sync.event = calloc(1, sizeof(InternalEvent));
> -                *grabinfo->sync.event = *event;
> +                    grabinfo->sync.event = calloc(1, sizeof(DeviceEvent));
> +                *grabinfo->sync.event = event->device_event;

separate patch please.

>  		grabinfo->sync.state = FROZEN_WITH_EVENT;
>              }
>  
> @@ -3609,7 +3635,8 @@ CheckDeviceGrabs(DeviceIntPtr device, DeviceEvent *event, WindowPtr ancestor)
>  	for (; i < focus->traceGood; i++)
>  	{
>  	    pWin = focus->trace[i];
> -	    if (CheckPassiveGrabsOnWindow(pWin, device, event, sendCore, TRUE))
> +	    if (CheckPassiveGrabsOnWindow(pWin, device, (InternalEvent *) event,
> +                                          sendCore, TRUE))

tab/space indentation mixup

>  		return TRUE;
>  	}
>  
> @@ -3622,7 +3649,8 @@ CheckDeviceGrabs(DeviceIntPtr device, DeviceEvent *event, WindowPtr ancestor)
>      for (; i < device->spriteInfo->sprite->spriteTraceGood; i++)
>      {
>  	pWin = device->spriteInfo->sprite->spriteTrace[i];
> -	if (CheckPassiveGrabsOnWindow(pWin, device, event, sendCore, TRUE))
> +	if (CheckPassiveGrabsOnWindow(pWin, device, (InternalEvent *) event,
> +                                      sendCore, TRUE))

tab/space indentation mixup


Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net> otherwise.

Cheers,
  Peter

>  	    return TRUE;
>      }
>  
> diff --git a/include/dix.h b/include/dix.h
> index 12e4b59..cab0213 100644
> --- a/include/dix.h
> +++ b/include/dix.h
> @@ -375,7 +375,7 @@ extern void ReleaseActiveGrabs(
>  extern GrabPtr CheckPassiveGrabsOnWindow(
>      WindowPtr /* pWin */,
>      DeviceIntPtr /* device */,
> -    DeviceEvent * /* event */,
> +    InternalEvent * /* event */,
>      BOOL /* checkCore */,
>      BOOL /* activate */);
>  
> -- 
> 1.7.2.3


More information about the xorg-devel mailing list