Reviewed-by: Jamey Sharp <ja...@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.hutte...@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 >
signature.asc
Description: Digital signature
_______________________________________________ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel