On Wed, Mar 24, 2010 at 03:13:10PM -0700, Ping Cheng wrote:
> We might not get all the valid types for ISDV4 devices in
> wcmPreInit(). Assuming users know what they are doing so we make their
> types valid.
> 
> Signed-off-by: Ping Cheng <[email protected]>

> From fcf8974b5f9ee3d515a12296467efc0a14b6ce82 Mon Sep 17 00:00:00 2001
> From: Ping Cheng <[email protected]>
> Date: Wed, 24 Mar 2010 13:54:32 -0700
> Subject: [PATCH] User defined ISDV4 device types in xorg.conf are always valid
> 
> We might not get all the valid types for ISDV4 devices in
> wcmPreInit(). Assuming users know what they are doing so we
> make their types valid.
> 
> Signed-off-by: Ping Cheng <[email protected]>
> ---
>  src/wcmValidateDevice.c |   28 +++++++++++++++++++++++++++-
>  1 files changed, 27 insertions(+), 1 deletions(-)
> 
> diff --git a/src/wcmValidateDevice.c b/src/wcmValidateDevice.c
> index f153cd2..f121fb1 100644
> --- a/src/wcmValidateDevice.c
> +++ b/src/wcmValidateDevice.c
> @@ -25,6 +25,7 @@
>  #include <sys/stat.h>
>  #include <fcntl.h>
>  #include <unistd.h>
> +#include <linux/serial.h>
>  
>  #define ARRAY_SIZE(a) (sizeof(a)/sizeof((a)[0]))
>  
> @@ -139,6 +140,10 @@ Bool wcmIsAValidType(LocalDevicePtr local, const char* 
> type)
>       int j, ret = FALSE;
>       WacomDevicePtr priv = (WacomDevicePtr)local->private;
>       WacomCommonPtr common = priv->common;
> +     char* dsource = xf86CheckStrOption(local->options, "_source", "");
> +     char* device = xf86SetStrOption(local->options, "Device", NULL);
> +     int fd = -1;
> +     struct serial_struct tmp;
>  
>       if (!type)
>               return FALSE;
> @@ -147,11 +152,32 @@ Bool wcmIsAValidType(LocalDevicePtr local, const char* 
> type)
>       for (j = 0; j < ARRAY_SIZE(wcmType); j++)
>       {
>               if (!strcmp(wcmType[j].type, type))
> -                     if (ISBITSET (common->wcmKeys, wcmType[j].tool))
> +             {
> +                     SYSCALL(fd = open(device, O_RDONLY));

file descriptor is leaked.

> +                     if (fd < 0)
>                       {
> +                             xf86Msg(X_WARNING, "%s: failed to open %s in "
> +                             "wcmIsAValidType.\n", local->name, device);
> +                             return FALSE;
> +                     }
> +
> +                     /* tool defined in xorg.conf and is an ISDV4 device? */
> +                     if (!strlen(dsource) && !ioctl(fd, TIOCGSERIAL, &tmp))
> +                     {
> +                             /* We might not get all the valid types for
> +                              * ISDV4 devices at this stage. Assuming user
> +                              * is right, let's make the type valid */
> +                             common->wcmKeys[LONG(wcmType[j].tool)]
> +                                      |= BIT(wcmType[j].tool);
>                               ret = TRUE;
>                               break;
>                       }
> +                     else if (ISBITSET (common->wcmKeys, wcmType[j].tool))
> +                     {
> +                             ret = TRUE;
> +                             break;
> +                     }
> +             }
>       }
>       return ret;
>  }
> -- 
> 1.6.6.1

thanks for the patch. A few comments:
This approach seems expensive. for USB devices we can already assume that
the keys are set correctly but with this patch we now open the FD on every
one of them, do some checks, etc. before checking the bits.
I think it'd be better to check the keys first and only if they are invalid,
then check the actual file. Having said that, this code seems in the wrong
place.

wcmIsAValidType() is called from only two locations - wcmNeedAutoHotplug()
and directly from wcmPreInit(). Only in the latter case are the checks above
sensible. Why not move the check into isdv4ProbeKeys(), since we do all the
opening of the device file, the check for serial, etc. there anyway.  It
should be trivial to add the bit flags for the device if it comes from the
xorg.conf.

Cheers,
  Peter

------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
_______________________________________________
Linuxwacom-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel

Reply via email to