[PATCH v2 02/11] dix: split out client delivery from DeliverEventsToWindow

Jamey Sharp jamey at minilop.net
Wed May 11 22:11:28 PDT 2011


Reviewed-by: Jamey Sharp <jamey at minilop.net>

I'm just going to trust you on the semantic change in the return value
here. :-)

Jamey

On Thu, May 12, 2011 at 11:43:26AM +1000, Peter Hutterer wrote:
> No real functional changes, this is just for improved readability.
> 
> DeliverEventsToWindow used to return an int to specify the number of
> deliveries (or rejected deliveries if negative). The number wasn't used by
> any caller other than for > 0 comparison.
> 
> This patch also changes the return value to be -1 or 1 even in case of
> multiple deliveries/rejections. The comment was updated accordingly.
> 
> A future patch should probably use the enum EventDeliveryState for
> DeliverEventsToWindow.
> 
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> ---
> On Wed, May 11, 2011 at 01:41:31PM -0700, Jamey Sharp wrote:
> > This doesn't look right, because only the last iteration of the loop
> > over other InputClients sets the EventDeliveryState. The original code
> > appears to have updated nondeliveries and deliveries for every entry in
> > the list of InputClients. Have I mis-read it?
> 
> good catch. not sure if we'd ever noticed this either. It would require two
> clients to register for XI events on the same window, a rather narrow
> use-case.
> 
> Changes to v1:
> - updated commit message
> - only return rejected we have no success for any client
> - update documentation for DeliverEventsToWindow to match current state
> 
>  dix/events.c |  131 +++++++++++++++++++++++++++++++++++++++-------------------
>  1 files changed, 88 insertions(+), 43 deletions(-)
> 
> diff --git a/dix/events.c b/dix/events.c
> index f61d0c3..783fd13 100644
> --- a/dix/events.c
> +++ b/dix/events.c
> @@ -1976,6 +1976,76 @@ DeliverToWindowOwner(DeviceIntPtr dev, WindowPtr win,
>      return EVENT_NOT_DELIVERED;
>  }
>  
> +/**
> + * Deliver events to clients registered on the window.
> + *
> + * @param client_return On successful delivery, set to the recipient.
> + * @param mask_return On successful delivery, set to the recipient's event
> + * mask for this event.
> + */
> +static enum EventDeliveryState
> +DeliverEventToClients(DeviceIntPtr dev, WindowPtr win, xEvent *events,
> +                      int count, Mask filter, GrabPtr grab,
> +                      ClientPtr *client_return, Mask *mask_return)
> +{
> +    int attempt;
> +    enum EventDeliveryState rc = EVENT_SKIP;
> +    InputClients *other;
> +
> +    if (CORE_EVENT(events))
> +        other = (InputClients *)wOtherClients(win);
> +    else if (XI2_EVENT(events))
> +    {
> +        OtherInputMasks *inputMasks = wOtherInputMasks(win);
> +        /* Has any client selected for the event? */
> +        if (!GetWindowXI2Mask(dev, win, events))
> +            goto out;
> +        other = inputMasks->inputClients;
> +    } else {
> +        OtherInputMasks *inputMasks = wOtherInputMasks(win);
> +        /* Has any client selected for the event? */
> +        if (!inputMasks ||
> +            !(inputMasks->inputEvents[dev->id] & filter))
> +            goto out;
> +
> +        other = inputMasks->inputClients;
> +    }
> +
> +    rc = EVENT_NOT_DELIVERED;
> +
> +    for (; other; other = other->next)
> +    {
> +        Mask mask;
> +
> +        if (IsInterferingGrab(rClient(other), dev, events))
> +            continue;
> +
> +        mask = GetEventMask(dev, events, other);
> +
> +        if (XaceHook(XACE_RECEIVE_ACCESS, rClient(other), win,
> +                    events, count))
> +            /* do nothing */;
> +        else if ( (attempt = TryClientEvents(rClient(other), dev,
> +                        events, count,
> +                        mask, filter, grab)) )
> +        {
> +            if (attempt > 0)
> +            {
> +                rc = EVENT_DELIVERED;
> +                *client_return = rClient(other);
> +                *mask_return = mask;
> +                /* Success overrides non-success, so if we've been
> +                 * successful on one client, return that */
> +            } else if (rc == EVENT_NOT_DELIVERED)
> +                rc = EVENT_REJECTED;
> +        }
> +    }
> +
> +out:
> +    return rc;
> +}
> +
> +
>  
>  /**
>   * Deliver events to a window. At this point, we do not yet know if the event
> @@ -1995,21 +2065,20 @@ DeliverToWindowOwner(DeviceIntPtr dev, WindowPtr win,
>   * @param filter Mask based on event type.
>   * @param grab Possible grab on the device that caused the event.
>   *
> - * @return Number of events delivered to various clients.
> + * @return a positive number if at least one successful delivery has been
> + * made, 0 if no events were delivered, or a negative number if the event
> + * has not been delivered _and_ rejected by at least one client.
>   */
>  int
>  DeliverEventsToWindow(DeviceIntPtr pDev, WindowPtr pWin, xEvent
>          *pEvents, int count, Mask filter, GrabPtr grab)
>  {
>      int deliveries = 0, nondeliveries = 0;
> -    int attempt;
> -    InputClients *other;
>      ClientPtr client = NullClient;
>      Mask deliveryMask = 0; /* If a grab occurs due to a button press, then
>  		              this mask is the mask of the grab. */
>      int type = pEvents->u.u.type;
>  
> -
>      /* Deliver to window owner */
>      if ((filter == CantBeFiltered) || CORE_EVENT(pEvents))
>      {
> @@ -2038,50 +2107,26 @@ DeliverEventsToWindow(DeviceIntPtr pDev, WindowPtr pWin, xEvent
>      /* CantBeFiltered means only window owner gets the event */
>      if (filter != CantBeFiltered)
>      {
> -        if (CORE_EVENT(pEvents))
> -            other = (InputClients *)wOtherClients(pWin);
> -        else if (XI2_EVENT(pEvents))
> -        {
> -            OtherInputMasks *inputMasks = wOtherInputMasks(pWin);
> -            /* Has any client selected for the event? */
> -            if (!GetWindowXI2Mask(pDev, pWin, pEvents))
> -                return 0;
> -            other = inputMasks->inputClients;
> -        } else {
> -            OtherInputMasks *inputMasks = wOtherInputMasks(pWin);
> -            /* Has any client selected for the event? */
> -            if (!inputMasks ||
> -                !(inputMasks->inputEvents[pDev->id] & filter))
> -                return 0;
> +        enum EventDeliveryState rc;
>  
> -            other = inputMasks->inputClients;
> -        }
> +        rc = DeliverEventToClients(pDev, pWin, pEvents, count, filter, grab,
> +                                   &client, &deliveryMask);
>  
> -        for (; other; other = other->next)
> +        switch(rc)
>          {
> -            Mask mask;
> -            if (IsInterferingGrab(rClient(other), pDev, pEvents))
> -                continue;
> -
> -            mask = GetEventMask(pDev, pEvents, other);
> -
> -            if (XaceHook(XACE_RECEIVE_ACCESS, rClient(other), pWin,
> -                        pEvents, count))
> -                /* do nothing */;
> -            else if ( (attempt = TryClientEvents(rClient(other), pDev,
> -                            pEvents, count,
> -                            mask, filter, grab)) )
> -            {
> -                if (attempt > 0)
> -                {
> -                    deliveries++;
> -                    client = rClient(other);
> -                    deliveryMask = mask;
> -                } else
> -                    nondeliveries--;
> -            }
> +            case EVENT_SKIP:
> +                return 0;
> +            case EVENT_REJECTED:
> +                nondeliveries--;
> +                break;
> +            case EVENT_DELIVERED:
> +                deliveries++;
> +                break;
> +            case EVENT_NOT_DELIVERED:
> +                break;
>          }
>      }
> +
>      /*
>       * Note that since core events are delivered first, an implicit grab may
>       * be activated on a core grab, stopping the XI events.
> -- 
> 1.7.4.4
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.x.org/archives/xorg-devel/attachments/20110511/2563a6d1/attachment.pgp>


More information about the xorg-devel mailing list