[PATCH 2/4] X event queue mutex

Peter Hutterer peter.hutterer at who-t.net
Mon Nov 3 21:06:49 PST 2008


Sorry for the late reply, I was tied up and missed the mail.

On Thu, Oct 23, 2008 at 02:47:30PM -0700, Jeremy Huddleston wrote:
> And here's a stab at setting up mieqProcessInputEvents in master to be  
> more friendly towards this locking.  master doesn't work for us on OSX, 
> so I can't really verify that it works... I may have missed an e-> to e. 
> or e->events[i]->event to event[i] somewhere...
>
> This also fixes what I think is a bug in the custom event handler...  
> right now we have:
>
> handler(DequeueScreen(e->pDev)->myNum, e->events->event,
>         e->pDev, e->nevents);
> if (!e->pDev->isMaster && e->pDev->u.master) {
>     handler(DequeueScreen(e->pDev->u.master)->myNum,
>             e->events->event, e->pDev->u.master, e->nevents);
> }
>
> But that fails if e->nevents > 1 ... granted all current custom event  
> handlers have nevents=1, but it's worth noting.

with the move of the handler procesing (dac9e918) this should now be.

handler(.., event, ...);
if (!e->pDev->isMaster && ...)
  handler(..., master_event, ...)

otherwise we're not protected against event manipulation in the handler.

> Also, why do we call the handler twice here?

Only one event is in the queue, but we need to process it twice, once as
coming from the slave device, once as coming from the master device. These two
are independent.


-        e = &miEventQueue.events[miEventQueue.head];
+        memcpy(&e, &miEventQueue.events[miEventQueue.head],  sizeof(EventRec));

if we're not using the EventRec as such anyway, it might just be better to
extract the things we need and improve readability.

+        event = xcalloc(e.nevents, sizeof(xEvent));

that doesn't work. GenericEvents may be larger than xEvent, hence the slightly
weird copying code. What you are doing here corrupts memory after the first
device switch. How about this one here, quick test shows it works:



More information about the xorg mailing list