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 <peter.hutte...@who-t.net> 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 <peter.hutte...@who-t.net> --- 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 Linuxwacom-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel