On Wed, 2012-04-18 at 14:57 +1000, Peter Hutterer wrote:
> Add a new type WacomMatch that holds device matching information.
> A WacomDevice in libwacom now has a bunch of possible matches that can be
> queried. The first possible match is the default match unless the device was
> updated otherwise (e.g. libwacom_device_new_from_path will set the correct
> match).
>
> Previous calls to get bustype, vendor_id, product_id now return the set
> match's values.
>
> Basic refcounting was added to the WacomDevice to allow for the device to be
> stored multiple times in the device hashtable.
>
> Signed-off-by: Peter Hutterer <[email protected]>
> ---
> data/generate-udev-rules.c | 18 +++++--
> libwacom/libwacom-database.c | 68 ++++++++++++++++++++-------
> libwacom/libwacom.c | 106
> +++++++++++++++++++++++++++++++++++-------
> libwacom/libwacom.h | 20 +++++++-
> libwacom/libwacomint.h | 16 +++++--
> 5 files changed, 185 insertions(+), 43 deletions(-)
>
> diff --git a/data/generate-udev-rules.c b/data/generate-udev-rules.c
> index 4c70fce..93f7bba 100644
> --- a/data/generate-udev-rules.c
> +++ b/data/generate-udev-rules.c
> @@ -41,11 +41,11 @@ static void print_udev_header (void)
> printf ("\n");
> }
>
> -static void print_udev_entry (WacomDevice *device)
> +static void print_udev_entry_for_match (WacomDevice *device, const
> WacomMatch *match)
> {
> - WacomBusType type = libwacom_get_bustype (device);
> - int vendor = libwacom_get_vendor_id (device);
> - int product = libwacom_get_product_id (device);
> + WacomBusType type = libwacom_match_get_bustype (match);
> + int vendor = libwacom_match_get_vendor_id (match);
> + int product = libwacom_match_get_product_id (match);
> int has_touch = libwacom_has_touch (device);
> static char *touchpad;
>
> @@ -67,6 +67,16 @@ static void print_udev_entry (WacomDevice *device)
> }
> }
>
> +static void print_udev_entry (WacomDevice *device)
> +{
> + WacomMatch **matches;
> + int nmatches;
> +
> + matches = libwacom_get_matches(device, &nmatches);
NULL terminate your matches instead, and use:
for (i = 0; matches[i] != NULL; i++)
If libwacom_get_matches() owns the array you're returning, I'd rather it
marked the return as const.
> + while(nmatches--)
> + print_udev_entry_for_match(device, matches[nmatches]);
> +}
> +
> static void print_udev_trailer (void)
> {
> printf ("\n");
> diff --git a/libwacom/libwacom-database.c b/libwacom/libwacom-database.c
> index 2a0ee19..dbe1738 100644
> --- a/libwacom/libwacom-database.c
> +++ b/libwacom/libwacom-database.c
> @@ -122,21 +122,45 @@ make_match_string (WacomBusType bus, int vendor_id, int
> product_id)
> }
>
> static int
> -libwacom_matchstr_to_ints(const char *match, uint32_t *vendor_id, uint32_t
> *product_id, WacomBusType *bus)
> +libwacom_matchstr_to_matches(WacomDevice *device, const char *match)
> {
> - char busstr[64];
> - int rc;
> + int rc = 1;
> + char **strs;
> + int i, nmatches = 0;
> + WacomBusType first_bus;
> + int first_vendor_id, first_product_id;
>
> if (match == NULL)
> return 0;
>
> - rc = sscanf(match, "%63[^:]:%x:%x", busstr, vendor_id, product_id);
> - if (rc != 3)
> - return 0;
> + strs = g_strsplit(match, ";", 0);
for (i = 0; strs[i] != NULL; i++)
saves us from running the whole length of the array every loop.
> + for (i = 0; i < g_strv_length(strs); i++) {
> + char busstr[64];
> + int vendor_id, product_id;
> + WacomBusType bus;
> + rc = sscanf(strs[i], "%63[^:]:%x:%x", busstr, &vendor_id,
> &product_id);
> + if (rc != 3) {
> + DBG("failed to match '%s' for product/vendor IDs.
> Skipping.\n", strs[i]);
> + continue;
> + }
> + bus = bus_from_str (busstr);
>
> - *bus = bus_from_str (busstr);
> + libwacom_update_match(device, bus, vendor_id, product_id);
>
> - return 1;
> + if (nmatches == 0) {
> + first_bus = bus;
> + first_vendor_id = vendor_id;
> + first_product_id = product_id;
> + }
> + nmatches++;
> + }
> +
> + /* set default to first entry */
> + if (nmatches > 1)
> + libwacom_update_match(device, first_bus, first_vendor_id,
> first_product_id);
> +
> + g_strfreev(strs);
> + return i;
> }
>
> static void
> @@ -298,18 +322,17 @@ libwacom_parse_tablet_keyfile(const char *path)
>
> match = g_key_file_get_string(keyfile, DEVICE_GROUP, "DeviceMatch",
> NULL);
> if (g_strcmp0 (match, GENERIC_DEVICE_MATCH) == 0) {
> - device->match = match;
> + libwacom_update_match(device, WBUSTYPE_UNKNOWN, 0, 0);
> } else {
> - if (!libwacom_matchstr_to_ints(match, &device->vendor_id,
> &device->product_id, &device->bus)) {
> + if (libwacom_matchstr_to_matches(device, match) == 0) {
> DBG("failed to match '%s' for product/vendor IDs in
> '%s'\n", match, path);
> g_free (match);
> g_free (device);
> device = NULL;
> goto out;
> }
> - device->match = make_match_string(device->bus,
> device->vendor_id, device->product_id);
> - g_free (match);
> }
> + g_free (match);
>
> device->name = g_key_file_get_string(keyfile, DEVICE_GROUP, "Name",
> NULL);
> device->width = g_key_file_get_integer(keyfile, DEVICE_GROUP, "Width",
> NULL);
> @@ -358,17 +381,17 @@ libwacom_parse_tablet_keyfile(const char *path)
>
> if (device->features & FEATURE_BUILTIN &&
> device->features & FEATURE_REVERSIBLE)
> - g_warning ("Tablet '%s' is both reversible and builtin. This is
> impossible", device->match);
> + g_warning ("Tablet '%s' is both reversible and builtin. This is
> impossible", libwacom_get_match(device));
>
> if (!(device->features & FEATURE_RING) &&
> (device->features & FEATURE_RING2))
> - g_warning ("Table '%s' has Ring2 but no Ring. This is
> impossible", device->match);
> + g_warning ("Table '%s' has Ring2 but no Ring. This is
> impossible", libwacom_get_match(device));
>
> device->num_strips = g_key_file_get_integer(keyfile, FEATURES_GROUP,
> "NumStrips", NULL);
> device->num_buttons = g_key_file_get_integer(keyfile, FEATURES_GROUP,
> "Buttons", &error);
> if (device->num_buttons == 0 &&
> g_error_matches (error, G_KEY_FILE_ERROR,
> G_KEY_FILE_ERROR_KEY_NOT_FOUND)) {
> - g_warning ("Tablet '%s' has no buttons defined, do something!",
> device->match);
> + g_warning ("Tablet '%s' has no buttons defined, do something!",
> libwacom_get_match(device));
> g_clear_error (&error);
> }
> if (device->num_buttons > 0) {
> @@ -445,13 +468,24 @@ libwacom_database_new_for_path (const char *datadir)
> nfiles = n;
> while(n--) {
> WacomDevice *d;
> + WacomMatch **matches;
> + int nmatches;
>
> path = g_build_filename (datadir, files[n]->d_name, NULL);
> d = libwacom_parse_tablet_keyfile(path);
> g_free(path);
>
> - if (d)
> - g_hash_table_insert (db->device_ht, g_strdup (d->match), d);
> + if (!d)
> + continue;
> +
> + matches = libwacom_get_matches(d, &nmatches);
Ditto, regarding the for loop.
> + while (nmatches--) {
> + const char *matchstr;
> +
> + matchstr =
> libwacom_match_get_match_string(matches[nmatches]);
> + g_hash_table_insert (db->device_ht, g_strdup (matchstr), d);
> + d->refcnt++;
> + }
> }
>
> while(nfiles--)
> diff --git a/libwacom/libwacom.c b/libwacom/libwacom.c
> index c16e4b5..a8fcc7e 100644
> --- a/libwacom/libwacom.c
> +++ b/libwacom/libwacom.c
> @@ -172,20 +172,34 @@ bail:
> return retval;
> }
>
> +static WacomMatch *libwacom_copy_match(const WacomMatch *src)
> +{
> + WacomMatch *dst;
> +
> + dst = g_new0(WacomMatch, 1);
> + *dst = *src;
Don't really like copying structure contents like this. Could you please
make the copying explicit. The clever compiler will deal with the rest.
> + dst->match = g_strdup(src->match);
> +
> + return dst;
> +}
> +
> static WacomDevice *
> libwacom_copy(const WacomDevice *device)
> {
> WacomDevice *d;
> + int i;
>
> d = g_new0 (WacomDevice, 1);
> + d->refcnt = 1;
Please use g_atomic_int_add() and other g_atomic_* functions for the
refcounting.
> d->name = g_strdup (device->name);
> d->width = device->width;
> d->height = device->height;
> - d->match = g_strdup (device->match);
> - d->vendor_id = device->vendor_id;
> - d->product_id = device->product_id;
> + d->nmatches = device->nmatches;
> + d->matches = g_malloc(d->nmatches * sizeof(WacomMatch*));
> + for (i = 0; i < d->nmatches; i++)
> + d->matches[i] = libwacom_copy_match(device->matches[i]);
> + d->match = device->match;
> d->cls = device->cls;
> - d->bus = device->bus;
> d->num_strips = device->num_strips;
> d->features = device->features;
> d->strips_num_modes = device->strips_num_modes;
> @@ -195,6 +209,7 @@ libwacom_copy(const WacomDevice *device)
> d->supported_styli = g_memdup (device->supported_styli, sizeof(int) *
> device->num_styli);
> d->num_buttons = device->num_buttons;
> d->buttons = g_memdup (device->buttons, sizeof(WacomButtonFlags) *
> device->num_buttons);
> + d->refcnt = 1;
> return d;
> }
>
> @@ -256,11 +271,13 @@ libwacom_new_from_path(WacomDeviceDatabase *db, const
> char *path, int fallback,
> g_free (ret->name);
> ret->name = name;
> }
> - libwacom_update_match(ret, bus, vendor_id, product_id);
> } else {
> g_free (name);
> }
>
> + /* for multiple-match devices, set to the one we requested */
> + libwacom_update_match(ret, bus, vendor_id, product_id);
> +
> if (device) {
> if (builtin == IS_BUILTIN_TRUE)
> ret->features |= FEATURE_BUILTIN;
> @@ -328,9 +345,18 @@ libwacom_new_from_name(WacomDeviceDatabase *db, const
> char *name, WacomError *er
> void
> libwacom_destroy(WacomDevice *device)
> {
> + int i;
> +
> + if (--device->refcnt)
> + return;
g_atomic_*
> g_free (device->name);
>
> - g_free (device->match);
> + for (i = 0; i < device->nmatches; i++) {
> + g_free (device->matches[i]->match);
> + g_free (device->matches[i]);
> + }
> + g_free (device->matches);
> g_free (device->supported_styli);
> g_free (device->buttons);
> g_free (device);
> @@ -339,16 +365,37 @@ libwacom_destroy(WacomDevice *device)
> void
> libwacom_update_match(WacomDevice *device, WacomBusType bus, int vendor_id,
> int product_id)
> {
> - device->vendor_id = vendor_id;
> - device->product_id = product_id;
> - device->bus = bus;
> - g_free(device->match);
> - device->match = make_match_string(device->bus, device->vendor_id,
> device->product_id);
> + char *newmatch;
> + int i;
> + WacomMatch match;
> +
> + if (bus == WBUSTYPE_UNKNOWN && vendor_id == 0 && product_id == 0)
> + newmatch = g_strdup("generic");
> + else
> + newmatch = make_match_string(bus, vendor_id, product_id);
> +
> + match.match = newmatch;
> + match.bus = bus;
> + match.vendor_id = vendor_id;
> + match.product_id = product_id;
> +
> + for (i = 0; i < device->nmatches; i++) {
> + if
> (g_strcmp0(libwacom_match_get_match_string(device->matches[i]), newmatch) ==
> 0) {
> + device->match = i;
> + g_free(newmatch);
> + return;
> + }
> + }
> +
> + device->matches = realloc(device->matches, ++device->nmatches *
> sizeof(WacomMatch));
We use g_free() to free the matches, so you need to use g_realloc.
g_realloc_n() might even make it nicer to look at.
> + device->matches[device->nmatches-1] = libwacom_copy_match(&match);
> + device->match = device->nmatches - 1;
> + g_free(newmatch);
> }
>
> int libwacom_get_vendor_id(WacomDevice *device)
> {
> - return device->vendor_id;
> + return device->matches[device->match]->vendor_id;
> }
Add guards around public API.
> const char* libwacom_get_name(WacomDevice *device)
> @@ -358,14 +405,18 @@ const char* libwacom_get_name(WacomDevice *device)
>
> int libwacom_get_product_id(WacomDevice *device)
> {
> - return device->product_id;
> + return device->matches[device->match]->product_id;
> }
Ditto.
> const char* libwacom_get_match(WacomDevice *device)
> {
> - /* FIXME make sure this only returns the first match
> - * when we implement multiple matching */
> - return device->match;
> + return device->matches[device->match]->match;
> +}
> +
> +WacomMatch** libwacom_get_matches(WacomDevice *device, int *nmatches)
> +{
> + *nmatches = device->nmatches;
> + return device->matches;
> }
Remove the nmatches.
> int libwacom_get_width(WacomDevice *device)
> @@ -447,7 +498,7 @@ int libwacom_is_reversible(WacomDevice *device)
>
> WacomBusType libwacom_get_bustype(WacomDevice *device)
> {
> - return device->bus;
> + return device->matches[device->match]->bus;
> }
>
> WacomButtonFlags
> @@ -519,4 +570,25 @@ void libwacom_stylus_destroy(WacomStylus *stylus)
> g_free (stylus);
> }
>
> +
> +WacomBusType libwacom_match_get_bustype(const WacomMatch *match)
> +{
> + return match->bus;
> +}
> +
> +uint32_t libwacom_match_get_product_id(const WacomMatch *match)
> +{
> + return match->product_id;
> +}
> +
> +uint32_t libwacom_match_get_vendor_id(const WacomMatch *match)
> +{
> + return match->vendor_id;
> +}
> +
> +const char* libwacom_match_get_match_string(const WacomMatch *match)
> +{
> + return match->match;
> +}
> +
> /* vim: set noexpandtab tabstop=8 shiftwidth=8: */
> diff --git a/libwacom/libwacom.h b/libwacom/libwacom.h
> index bdc12b0..3fe02dd 100644
> --- a/libwacom/libwacom.h
> +++ b/libwacom/libwacom.h
> @@ -34,6 +34,7 @@
> #define _LIBWACOM_H_
> /** @endcond */
>
> +#include <stdint.h>
> /**
> @mainpage
>
> @@ -79,6 +80,8 @@
>
> typedef struct _WacomDevice WacomDevice;
>
> +typedef struct _WacomMatch WacomMatch;
> +
> typedef struct _WacomStylus WacomStylus;
>
> typedef struct _WacomError WacomError;
> @@ -291,12 +294,21 @@ int libwacom_get_vendor_id(WacomDevice *device);
>
> /**
> * @param device The tablet to query
> - * @return The first match for the device in question
> + * @return The current match string used for this device (if set) or the
> first
> + * match string in the tablet definition.
> */
> const char* libwacom_get_match(WacomDevice *device);
>
> /**
> * @param device The tablet to query
> + * @param nmatches The number of matches possible on this device.
> + * @return A pointer to the list of possible matches for this device. Do not
> + * modify this pointer or any content!
> + */
> +WacomMatch** libwacom_get_matches(WacomDevice *device, int *nmatches);
> +
> +/**
> + * @param device The tablet to query
> * @return The numeric product ID for this device
> */
> int libwacom_get_product_id(WacomDevice *device);
> @@ -464,6 +476,12 @@ int libwacom_stylus_has_lens (const WacomStylus
> *stylus);
> */
> WacomStylusType libwacom_stylus_get_type (const WacomStylus *stylus);
>
> +
> +WacomBusType libwacom_match_get_bustype(const WacomMatch *match);
> +uint32_t libwacom_match_get_product_id(const WacomMatch *match);
> +uint32_t libwacom_match_get_vendor_id(const WacomMatch *match);
> +const char* libwacom_match_get_match_string(const WacomMatch *match);
> +
> #endif /* _LIBWACOM_H_ */
>
> /* vim: set noexpandtab tabstop=8 shiftwidth=8: */
> diff --git a/libwacom/libwacomint.h b/libwacom/libwacomint.h
> index 960bb3c..6be561b 100644
> --- a/libwacom/libwacomint.h
> +++ b/libwacom/libwacomint.h
> @@ -70,6 +70,13 @@ enum WacomFeature {
> FEATURE_REVERSIBLE = (1 << 5)
> };
>
> +struct _WacomMatch {
> + char *match;
> + WacomBusType bus;
> + uint32_t vendor_id;
> + uint32_t product_id;
> +};
> +
> /* WARNING: When adding new members to this struct
> * make sure to update libwacom_copy() ! */
> struct _WacomDevice {
> @@ -77,12 +84,11 @@ struct _WacomDevice {
> int width;
> int height;
>
> - char *match;
> - uint32_t vendor_id;
> - uint32_t product_id;
> + int match; /* used match or first match by default */
> + WacomMatch **matches;
> + int nmatches;
>
> WacomClass cls;
> - WacomBusType bus;
> int num_strips;
> uint32_t features;
>
> @@ -95,6 +101,8 @@ struct _WacomDevice {
>
> int num_buttons;
> WacomButtonFlags *buttons;
> +
> + int refcnt; /* for the db hashtable */
> };
>
> struct _WacomStylus {
------------------------------------------------------------------------------
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
[email protected]
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
