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

Peter Hutterer peter.hutterer at who-t.net
Mon Jul 27 18:49:28 PDT 2009


Thanks for the update.

On Mon, Jul 27, 2009 at 12:06:14PM +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.

I'd rather you skip this paragraph in the commit message since it doesn't
contribute majorly. When you're sending a git patch you can add comments
like this after the --- deliminator (before the diffstat) and it will get
skipped when the patch is applied.

This space is also handy when you're resending a patch as it allows you to
comment what has changed since the previous version.

Also one minor thing: I prefer "Add foo" rather than "Added foo" since
you're describing a patch that actively does something. Feel free to
disagree with that, I don't mind leaving it like that either.

anyway, the real comment is below.
 
> xf86PostKeyboardEvent also makes use of xf86PostKeyEventP to avoid code
> duplication, and the valuator verification has been split into a
> separate static inline function, xf86VerifyValuators.
> ---
>  hw/xfree86/common/xf86Xinput.c |  157 ++++++++++++++++++++++++++--------------
>  hw/xfree86/common/xf86Xinput.h |    8 ++
>  2 files changed, 109 insertions(+), 56 deletions(-)
> 
> diff --git a/hw/xfree86/common/xf86Xinput.c b/hw/xfree86/common/xf86Xinput.c
> index b4169cf..3b0bf58 100644
> --- a/hw/xfree86/common/xf86Xinput.c
> +++ b/hw/xfree86/common/xf86Xinput.c
> @@ -701,6 +701,18 @@ DeleteInputDeviceRequest(DeviceIntPtr pDev)
>   * convenient functions to post events
>   */
>  
> +static inline Bool
> +xf86VerifyValuators(int num_valuators)
> +{
> +    if (num_valuators > MAX_VALUATORS) {
> +	xf86Msg(X_ERROR, "%s: num_valuator %d is greater than"
> +		" MAX_VALUATORS\n", __FUNCTION__, num_valuators);

wouldn't __FUNCTION__ resolve to xf86VerifyValuators? If so, that seems a
tad useless. Better to make it a macro so it resolves to the actual
xf86PostXYZ function when triggered.

Cheers,
  Peter


More information about the xorg-devel mailing list