[PATCH evdev] Don't delete the device on ENODEV

Peter Hutterer peter.hutterer at who-t.net
Mon Aug 6 14:37:35 PDT 2012


On Mon, Aug 06, 2012 at 09:43:33AM -0700, Chase Douglas wrote:
> On 07/31/2012 08:15 PM, Peter Hutterer wrote:
> >This is signal handler code and we cannot clean up properly while in the
> >signal handler. So reduce the code to removing the signal handler and let
> >the device be cleaned up later.
> >
> >If hotplugging is on, the server will remove it when the config backend says
> >so and if it is off, the server will remove it on shutdown.
> >
> >Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> >---
> >  src/evdev.c |    6 +-----
> >  1 file changed, 1 insertion(+), 5 deletions(-)
> >
> >diff --git a/src/evdev.c b/src/evdev.c
> >index f54b66f..b832d98 100644
> >--- a/src/evdev.c
> >+++ b/src/evdev.c
> >@@ -1113,12 +1113,8 @@ EvdevReadInput(InputInfoPtr pInfo)
> >          if (len <= 0)
> >          {
> >              if (errno == ENODEV) /* May happen after resume */
> >-            {
> >-                EvdevMBEmuFinalize(pInfo);
> >-                Evdev3BEmuFinalize(pInfo);
> >                  xf86RemoveEnabledDevice(pInfo);
> >-                EvdevCloseDevice(pInfo);
> >-            } else if (errno != EAGAIN)
> >+            else if (errno != EAGAIN)
> >              {
> >                  /* We use X_NONE here because it doesn't alloc */
> >                  xf86MsgVerb(X_NONE, 0, "%s: Read error: %s\n", pInfo->name,
> >
> 
> I assume this caused a bug somehow? Please detail for the curious :).

oh, right. sorry. I triggered this condition with a uinput device that spews
events from the creation (cannot be reproduced easily with evemu, the server
was still inside NIDR when it started processing events). Evdev3BEmuFinalize
calls TimerFree() which then triggered the SIGIO bug mentioned in the other
thread.
http://lists.freedesktop.org/archives/xorg-devel/2012-August/033125.html

Plus, TimerFree calls free(), which shouldn't be done inside the signal
handler.

Cheers,
   Peter

> Either way, the reasoning is sound.
> 
> Reviewed-by: Chase Douglas <chase.douglas at canonical.com>


More information about the xorg-devel mailing list