On Wed, 2012-04-18 at 14:57 +1000, Peter Hutterer wrote:
> Bluetooth devices tagged with ID_INPUT_TABLET aren't tagged on the device,
> but on the parent instead. Match the parent for a tag before bailing out.
> 
> Signed-off-by: Peter Hutterer <peter.hutte...@who-t.net>
> ---
>  libwacom/libwacom.c |   21 +++++++++++++++------
>  1 file changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/libwacom/libwacom.c b/libwacom/libwacom.c
> index f8d104a..e54175d 100644
> --- a/libwacom/libwacom.c
> +++ b/libwacom/libwacom.c
> @@ -50,7 +50,7 @@ get_device_info (const char   *path,
>                WacomError   *error)
>  {
>       GUdevClient *client;
> -     GUdevDevice *device;
> +     GUdevDevice *device, *parent = NULL;

I don't like assignment at the same time as declaration.

>       const char * const subsystems[] = { "input", NULL };
>       gboolean retval;
>       const char *bus_str;
> @@ -71,10 +71,17 @@ get_device_info (const char   *path,
>       /* Touchpads are only for the "Finger" part of Bamboo devices */
>       if (g_udev_device_get_property_as_boolean (device, "ID_INPUT_TABLET") 
> == FALSE &&
>           g_udev_device_get_property_as_boolean (device, "ID_INPUT_TOUCHPAD") 
> == FALSE) {
> -             libwacom_error_set(error, WERROR_INVALID_PATH, "Device '%s' is 
> not a tablet", path);
> -             goto bail;
> +             parent = g_udev_device_get_parent(device);
> +             if (!parent || (g_udev_device_get_property_as_boolean (parent, 
> "ID_INPUT_TABLET") == FALSE &&
> +                             g_udev_device_get_property_as_boolean (parent, 
> "ID_INPUT_TOUCHPAD") == FALSE)) {
> +                             libwacom_error_set(error, WERROR_INVALID_PATH, 
> "Device '%s' is not a tablet", path);
> +                             goto bail;

And the 2 get_prop_as_boolean calls could do with moving to helper
function to clean up the indentation.

Isn't it a udev bug that the device doesn't get tagged?

Maybe we should move the bus check before the device type checks so that
we only ever poke the parent for Bluetooth devices, not for USB ones.

> +             }
>       }
>  
> +     /* FIXME: ID_BUS on the device is usb even for bluetooth devices,
> +      * but ID_BUS on the parent is NULL.

Did you test this with the Intuos4 WL? If so, it might be missing a call
saying that it's Bluetooth. It certainly showed up as Bluetooth for my
old Graphire Wireless.

> +      */
>       bus_str = g_udev_device_get_property (device, "ID_BUS");
>       /* Serial devices are weird */
>       if (bus_str == NULL) {
> @@ -83,12 +90,12 @@ get_device_info (const char   *path,
>       }
>       /* Poke the parent device for Bluetooth models */
>       if (bus_str == NULL) {
> -             GUdevDevice *parent;
> -
> -             parent = g_udev_device_get_parent (device);
> +             if (!parent)
> +                     parent = g_udev_device_get_parent (device);

I'd rather we didn't reused the parent like that. It's cheap enough to
instantiate, so I would unref it in the previous use, and reinstantiate
it here.

>               g_object_unref (device);
>               device = parent;
> +             parent = NULL;
>               bus_str = "bluetooth";
>       }
>  
> @@ -165,6 +172,8 @@ get_device_info (const char   *path,
>  bail:
>       if (retval == FALSE)
>               g_free (*name);
> +     if (parent != NULL)
> +             g_object_unref (parent);
>       if (device != NULL)
>               g_object_unref (device);
>       if (client != NULL)



------------------------------------------------------------------------------
Better than sec? Nothing is better than sec when it comes to
monitoring Big Data applications. Try Boundary one-second 
resolution app monitoring today. Free.
http://p.sf.net/sfu/Boundary-dev2dev
_______________________________________________
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel

Reply via email to