[RFC] xf86Xinput: Added the xf86Post(Proximity|Button|Key)EventP helper functions.

[Oliver McFadden]

On Mon, 2009-07-27 at 02:58 +0200, ext Peter Hutterer wrote:
> On Thu, Jul 23, 2009 at 02:33:24PM +0300, oliver.mcfadden at nokia.com wrote:
> > From: Oliver McFadden <oliver.mcfadden at nokia.com>
> > 
> > I only intend to use xf86PostButtonEventP in xf86-input-evdev, however,
> > for the sake of symmetry (and possible future use) I have added pointer
> > versions of all the VA args functions.
> 
> agreed in principle but a few comments remain:
> 
> > ---
> >  hw/xfree86/common/xf86Xinput.c |   94 ++++++++++++++++++++++++++++++++++++---
> >  hw/xfree86/common/xf86Xinput.h |    8 +++
> >  2 files changed, 94 insertions(+), 8 deletions(-)
> > 
> > diff --git a/hw/xfree86/common/xf86Xinput.c b/hw/xfree86/common/xf86Xinput.c
> > index b4169cf..5689b3e 100644
> > --- a/hw/xfree86/common/xf86Xinput.c
> > +++ b/hw/xfree86/common/xf86Xinput.c
> > @@ -800,7 +800,7 @@ xf86PostProximityEvent(DeviceIntPtr	device,
> >                         ...)
> >  {
> >      va_list var;
> > -    int i, nevents;
> > +    int i;
> >      int valuators[MAX_VALUATORS];
> >  
> >  
> > @@ -815,6 +815,27 @@ xf86PostProximityEvent(DeviceIntPtr	device,
> >          valuators[i] = va_arg(var, int);
> >      va_end(var);
> >  
> > +    xf86PostProximityEventP(device, is_in, first_valuator, num_valuators,
> > +			    valuators);
> > +
> > +}
> > +
> > +void
> > +xf86PostProximityEventP(DeviceIntPtr	device,
> > +                        int		is_in,
> > +                        int		first_valuator,
> > +                        int		num_valuators,
> > +                        int		*valuators)
> > +{
> > +    int i, nevents;
> > +
> > +
> > +    if (num_valuators > MAX_VALUATORS) {
> > +	xf86Msg(X_ERROR, "%s: num_valuator %d is greater than"
> > +	    " MAX_VALUATORS\n", __FUNCTION__, num_valuators);
> > +	return;
> > +    }
> > +
> >      GetEventList(&xf86Events);
> >      nevents = GetProximityEvents(xf86Events, device,
> >                                   is_in ? ProximityIn : ProximityOut, 
> > @@ -835,7 +856,7 @@ xf86PostButtonEvent(DeviceIntPtr	device,
> >  {
> >      va_list var;
> >      int valuators[MAX_VALUATORS];
> > -    int i = 0, nevents = 0;
> > +    int i = 0;
> >      int index;
> >  
> >  #if XFreeXDGA
> 
> why leave all the checks in xf86PostButtonEvent? afaict they can just be
> ripped out to avoid code duplication. same goes for the other fucntions.

The reason is that you may call either the VA args function, or the
pointer function. So they both need to do the verification.

I suppose the verification could be moved into a separate (static)
function and all the xf86Post...Event functions could call that. It
might reduce the added lines of code a little.

> > @@ -856,6 +877,36 @@ xf86PostButtonEvent(DeviceIntPtr	device,
> >          valuators[i] = va_arg(var, int);
> >      va_end(var);
> >  
> > +    xf86PostButtonEventP(device, is_absolute, button, is_down, first_valuator,
> > +			 num_valuators, valuators);
> > +
> > +}
> > +
> > +void
> > +xf86PostButtonEventP(DeviceIntPtr	device,
> > +                     int		is_absolute,
> > +                     int		button,
> > +                     int		is_down,
> > +                     int		first_valuator,
> > +                     int		num_valuators,
> > +                     int		*valuators)
> > +{
> > +    int i = 0, nevents = 0;
> > +    int index;
> > +
> > +#if XFreeXDGA
> > +    if (miPointerGetScreen(device)) {
> > +        index = miPointerGetScreen(device)->myNum;
> > +        if (DGAStealButtonEvent(device, index, button, is_down))
> > +            return;
> > +    }
> > +#endif
> > +    if (num_valuators > MAX_VALUATORS) {
> > +	xf86Msg(X_ERROR, "%s: num_valuator %d is greater than"
> > +	    " MAX_VALUATORS\n", __FUNCTION__, num_valuators);
> > +	return;
> > +    }
> > +
> >      GetEventList(&xf86Events);
> >      nevents = GetPointerEvents(xf86Events, device,
> >                                 is_down ? ButtonPress : ButtonRelease, button,
> > @@ -877,7 +928,7 @@ xf86PostKeyEvent(DeviceIntPtr	device,
> >                   ...)
> >  {
> >      va_list var;
> > -    int i = 0, nevents = 0;
> > +    int i = 0;
> >      static int valuators[MAX_VALUATORS];
> >  
> >      /* instil confidence in the user */
> > @@ -891,12 +942,39 @@ xf86PostKeyEvent(DeviceIntPtr	device,
> >  	return;
> >      }
> >  
> > -    if (is_absolute) {
> > -        va_start(var, num_valuators);
> > -        for (i = 0; i < num_valuators; i++)
> > -            valuators[i] = va_arg(var, int);
> > -        va_end(var);
> > +    va_start(var, num_valuators);
> > +    for (i = 0; i < num_valuators; i++)
> > +      valuators[i] = va_arg(var, int);
> > +    va_end(var);
> >  
> > +    xf86PostKeyEventP(device, key_code, is_down, is_absolute, first_valuator,
> > +		      num_valuators, valuators);
> > +
> > +}
> > +
> > +void
> > +xf86PostKeyEventP(DeviceIntPtr	device,
> > +                  unsigned int	key_code,
> > +                  int		is_down,
> > +                  int		is_absolute,
> > +                  int		first_valuator,
> > +                  int		num_valuators,
> > +                  int		*valuators)
> 
> The addition of this should make it possible to simplify
> xf86PostKeyboardEvent since we can now just call xf86PostKeyEventP with zero
> valuators. Can you pleese provide a follow-up patch for this (or merge it
> into this one)?

Yes, I'll do that.

> > +{
> > +    int i = 0, nevents = 0;
> > +
> > +    /* instil confidence in the user */
> > +    DebugF("this function has never been tested properly.  if things go quite "
> > +           "badly south after this message, then xf86PostKeyEvent is "
> > +           "broken.\n");
> > +
> > +    if (num_valuators > MAX_VALUATORS) {
> > +	xf86Msg(X_ERROR, "%s: num_valuator %d is greater than"
> > +	    " MAX_VALUATORS\n", __FUNCTION__, num_valuators);
> > +	return;
> > +    }
> > +
> > +    if (is_absolute) {
> >          GetEventList(&xf86Events);
> >          nevents = GetKeyboardValuatorEvents(xf86Events, device,
> >                                              is_down ? KeyPress : KeyRelease,
> > diff --git a/hw/xfree86/common/xf86Xinput.h b/hw/xfree86/common/xf86Xinput.h
> > index 0ad5664..f8eb24d 100644
> > --- a/hw/xfree86/common/xf86Xinput.h
> > +++ b/hw/xfree86/common/xf86Xinput.h
> > @@ -158,12 +158,20 @@ extern _X_EXPORT void xf86PostMotionEventP(DeviceIntPtr device, int is_absolute,
> >  			 int first_valuator, int num_valuators, int *valuators);
> >  extern _X_EXPORT void xf86PostProximityEvent(DeviceIntPtr device, int is_in,
> >  			    int first_valuator, int num_valuators, ...);
> > +extern _X_EXPORT void xf86PostProximityEventP(DeviceIntPtr device, int is_in, int first_valuator,
> > +			     int num_valuators, int *valuators);
> >  extern _X_EXPORT void xf86PostButtonEvent(DeviceIntPtr device, int is_absolute, int button,
> >  		    	 int is_down, int first_valuator, int num_valuators,
> >  			 ...);
> > +extern _X_EXPORT void xf86PostButtonEventP(DeviceIntPtr device, int is_absolute, int button,
> > +			  int is_down, int first_valuator, int num_valuators,
> > +			  int *valuators);
> >  extern _X_EXPORT void xf86PostKeyEvent(DeviceIntPtr device, unsigned int key_code, int is_down,
> >  		      int is_absolute, int first_valuator, int num_valuators,
> >  		      ...);
> > +extern _X_EXPORT void xf86PostKeyEventP(DeviceIntPtr device, unsigned int key_code, int is_down,
> > +		       int is_absolute, int first_valuator, int num_valuators,
> > +		       int *valuators);
> >  extern _X_EXPORT void xf86PostKeyboardEvent(DeviceIntPtr device, unsigned int key_code,
> >                             int is_down);
> >  extern _X_EXPORT int xf86ActivateDevice(LocalDevicePtr local);
> > -- 
> > 1.6.1
> > 
> Cheers,
>   Peter