On Wed, Jan 30, 2013 at 02:03:59PM +0100, Olivier Fourdan wrote:
>
> Hey PEter,
>
> Peter Hutterer said the following on 01/30/2013 05:27 AM:
> >No two device description files should match on the same ID.
> >
> >Since a tablet may be listed multiple times in the database if it has more
> >than one match, we can't assume the matches are unique, we need to compare
> >the device name.
> >
> >Build hashtable of {match string : device name}, then check for each new
> >existing match string if the device name is identical.
>
> I tried that patch by adding both the new definition from Pander for
> the CTH-661 and my own patch, expecting it to detect the duplicate
> definition and bail out (see the thread about adding support for the
> "libwacom CTH-661/L" and the discussion with Bastien).
>
> The case here is:
>
> - A tablet definition listing mutliple matches,
> DeviceMatch=usb:056a:00d3;usb:056a:00db under the name Name=Wacom
> Bamboo 2FG 6x8
> - Another tablet definition adding a single duplicate match,
> DeviceMatch=usb:056a:00db under a dfferent name, Name=Wacom Bamboo
> 4FG 6x8 SE NL
>
> I was expecting this to trigger a failure. But it did not happen,
> "make check" ended happily ever after, so I wondered why.
hah, you're right. somehow all my test-cases used the duplicated on a
multiple match and didn't pick up this simple case.
> When loading a tablet definition, libwacom_database_new_for_path()
> will add all the matches given for that tablet in a hash table whose
> key is the match.
>
> So if the duplicate file is loaded first, its entry in the hash
> table will get replaced by the other definition loaded later, no
> dupe in database, "make check" is happy (while I was expecting it
> not to be).
>
> If the duplicate file is loaded after, then only the match for that
> given /other/ entry is replaced, "make check" should fail.
>
> So I think this adding this check here is not sufficient. Ideally,
> this should be done when loading the database (also because people
> could add new definitions without rebuilding and running "make
> check").
right, and that's where we have to do it anyway since we otherwise won't
notice (as one gets overwritten). How about the following patch then?
I opted against g_assert() to give the caller a chance to clean up before
exiting rather than just killing it.
>From 330e89be2713d483ad72ef6b72743588f14c68b2 Mon Sep 17 00:00:00 2001
From: Peter Hutterer <[email protected]>
Date: Thu, 31 Jan 2013 13:52:39 +1000
Subject: [PATCH libwacom] libwacom: bail out if a duplicate match string is
found
If two devices use the same match string, something is wrong. Log a critical
error and return a NULL database.
Signed-off-by: Peter Hutterer <[email protected]>
---
libwacom/libwacom-database.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/libwacom/libwacom-database.c b/libwacom/libwacom-database.c
index 9813a4e..3eb65df 100644
--- a/libwacom/libwacom-database.c
+++ b/libwacom/libwacom-database.c
@@ -564,6 +564,13 @@ libwacom_database_new_for_path (const char *datadir)
for (match = matches; *match; match++) {
const char *matchstr;
matchstr = libwacom_match_get_match_string(*match);
+ /* no duplicate matches allowed */
+ if (g_hash_table_contains(db->device_ht, matchstr)) {
+ g_critical("Duplicate match of '%s' on device
'%s'.",
+ matchstr, libwacom_get_name(d));
+ libwacom_database_destroy(db);
+ return NULL;
+ }
g_hash_table_insert (db->device_ht, g_strdup (matchstr), d);
d->refcnt++;
}
--
1.8.1
------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_d2d_jan
_______________________________________________
Linuxwacom-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
