[PATCH/RFC] xfree86: add a hook to replace the new console handler.

[Peter Hutterer]

On Mon, Sep 06, 2010 at 07:20:52PM +0300, Tiago Vignatti wrote:
> On Mon, Sep 06, 2010 at 03:56:31AM +0200, ext Peter Hutterer wrote:
> > This hook is only necessary for the keyboard driver to remove the race
> > condition between drain_console() and the driver's ReadInput (Bug 29969).
> > 
> > The idea is that a driver that needs to handle events from the console
> > calls xf86ReplaceConsoleHandler() with it's own ReadInput (or NULL) and thus
> > removes the drain_console call. It's the driver's responsibility to restore
> > the previous behaviour when the driver is unloaded.
> > 
> > Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> > CC: Thomas Hellstrom <thellstrom at vmware.com>
> > ---
> > compile-tested only but something like this may fix the issue, given the
> > matching code in the keyboard driver?
> > 
> > since we haven't bumped the API yet for 1.10 and I've got a lot of changes
> > anyway, queing this one up for input abi 12 would be easy.
> > 
> >  hw/xfree86/common/xf86.h               |    1 +
> >  hw/xfree86/common/xf86Events.c         |   19 +++++++++++++++++++
> >  hw/xfree86/os-support/linux/lnx_init.c |    4 +---
> >  3 files changed, 21 insertions(+), 3 deletions(-)
> > 
> > diff --git a/hw/xfree86/common/xf86.h b/hw/xfree86/common/xf86.h
> > index 886c25b..ad271f1 100644
> > --- a/hw/xfree86/common/xf86.h
> > +++ b/hw/xfree86/common/xf86.h
> > @@ -208,6 +208,7 @@ extern _X_EXPORT void xf86InterceptSigIll(void (*sigillhandler)(void));
> >  extern _X_EXPORT Bool xf86EnableVTSwitch(Bool new);
> >  extern _X_EXPORT void xf86ProcessActionEvent(ActionEvent action, void *arg);
> >  extern _X_EXPORT void xf86PrintBacktrace(void);
> > +extern _X_EXPORT InputHandlerProc xf86SetConsoleHandler(InputHandlerProc handler, pointer data);
> 
> xf86Xinput.h isn't more appropriated? 

the other handler functions are in xf86.h as well so I figured I tack it on
here. I can move it up to make it more obvious in the diff (i.e.
xf86SetConsoleHandler is after xf86AddGeneralHandler)
 
  
> >  /* xf86Helper.c */
> >  
> > diff --git a/hw/xfree86/common/xf86Events.c b/hw/xfree86/common/xf86Events.c
> > index 2e82848..8b6cd2b 100644
> > --- a/hw/xfree86/common/xf86Events.c
> > +++ b/hw/xfree86/common/xf86Events.c
> > @@ -600,6 +600,25 @@ xf86AddGeneralHandler(int fd, InputHandlerProc proc, pointer data)
> >      return ih;
> >  }
> >  
> > +/**
> > + * Set the handler for the console's fd. Replaces (and returns) the previous
> > + * handler or NULL, whichever appropriate.
> > + */
> > +InputHandlerProc
> > +xf86SetConsoleHandler(InputHandlerProc proc, pointer data)
> > +{
> > +    static InputHandlerProc handler = NULL;
> > +    InputHandlerProc old_handler = handler;
> > +
> > +    if (old_handler)
> > +        xf86RemoveGeneralHandler(old_handler);
> > +
> > +    xf86AddGeneralHandler(xf86Info.consoleFd, proc, data);
> > +    handler = proc;
> > +
> > +    return old_handler;
> > +}
> > +
> >  static void
> >  removeInputHandler(IHPtr ih)
> >  {
> > diff --git a/hw/xfree86/os-support/linux/lnx_init.c b/hw/xfree86/os-support/linux/lnx_init.c
> > index 92bfde4..bf61ceb 100644
> > --- a/hw/xfree86/os-support/linux/lnx_init.c
> > +++ b/hw/xfree86/os-support/linux/lnx_init.c
> > @@ -277,9 +277,7 @@ xf86OpenConsole(void)
> >              tcsetattr(xf86Info.consoleFd, TCSANOW, &nTty);
> >  
> >              /* need to keep the buffer clean, else the kernel gets angry */
> > -	    if (xf86Info.allowEmptyInput)
> > -		console_handler = xf86AddGeneralHandler(xf86Info.consoleFd,
> > -							drain_console, NULL);
> > +	    xf86SetConsoleHandler(drain_console, NULL);
> >  
> >  	    /* we really should have a InitOSInputDevices() function instead
> >  	     * of Init?$#*&Device(). So I just place it here */
> > -- 
> 
> For the patch: 
> Reviewed-by: Tiago Vignatti <tiago.vignatti at nokia.com>

thanks.

> 
> BTW, this one also mess Fernando's patchset: 
> 
> http://lists.x.org/archives/xorg-devel/2010-August/012470.html

I'll try to get to it, it's been on my todo list for too long, sorry.

Cheers,
  Peter