libwacom: allow for duplicates across data directories

See #379, there is a use-case for overriding tablet files with existing
matches. HUION devices re-use USB IDs but apparently also change those IDs
within the same product line - causing clashes with other devices.

Right now our duplicate detection prevents any tablet file to have the same
match as another one, so a user would have to change two files to get the
right match - the one for the device needs the match added and the wrong
tablet file needs the match removed.

Fix this by allowing duplicate files across data directories and setting /etc
as preferred: it's now possible to drop a .tablet file into /etc/libwacom and
have that override any matching file from /usr/share.

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>

4 files changed, 98 insertions, 9 deletions

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 4b1d4dc..07d6e42 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -172,6 +172,52 @@ jobs:
       - name: compare device database
         run: diff -u8 devicelist.default.txt devicelist.modified.txt
 
+  ####
+  # duplicate device check
+  duplicate-devices:
+    needs: meson
+    runs-on: ubuntu-20.04
+    steps:
+      - uses: actions/checkout@v2
+      # install python so we get pip for meson
+      - uses: actions/setup-python@v1
+        with:
+          python-version: '3.8'
+      # Run as sudo because we install to /etc and thus need the pip
+      # packages available to root
+      - uses: ./.github/actions/pkginstall
+        with:
+          apt: $UBUNTU_PACKAGES
+          pip: $PIP_PACKAGES
+          pip_precmd: sudo
+      - uses: ./.github/actions/meson
+        with:
+          meson_args: --prefix=/usr
+          meson_skip_test: yes
+          ninja_args: install
+          ninja_precmd: sudo
+      - name: list devices with database in /usr
+        run: libwacom-list-devices --format=oneline > devicelist.default.txt
+      - run: sudo mkdir /etc/libwacom
+        # We override a Cintiq 27QHD with a single device match, and one of
+        # the multiple matches of the Intuos Pro L.
+      - name: copy and modify tablet files to override another one
+        run: |
+          sed -e 's/27QHD/27QHD MODIFIED/' data/cintiq-27hd.tablet | sudo tee /etc/libwacom/modified-cintiq.tablet
+          sed -e 's/Pro L/Pro L MODIFIED/' -e 's/usb:056a:0358;//' data/intuos-pro-2-l.tablet | sudo tee /etc/libwacom/modified-intuos.tablet
+      - name: list all devices for debugging
+        run: libwacom-list-devices --format=oneline
+
+      # We expect the modified tablets to be listed
+      # We expect the remaining match for a modified tablet to be listed
+      # We expect the overridden match *not* to be listed
+      - name: check for the expected devices to be present (or not present)
+        run: |
+          libwacom-list-devices --format=oneline | grep "usb:056a:032a:Wacom Cintiq 27QHD MODIFIED"
+          libwacom-list-devices --format=oneline | grep "bluetooth:056a:0361:Wacom Intuos Pro L MODIFIED"
+          test $(libwacom-list-devices --format=oneline | grep "usb:056a:0358" | wc -l) -eq 1
+          test $(libwacom-list-devices --format=oneline | grep "usb:056a:032a:Wacom Cintiq 27QHD M" | wc -l) -eq 1
+
   ###
   #
   # tarball verification
diff --git a/libwacom/libwacom-database.c b/libwacom/libwacom-database.c
index b8397fb..30513fc 100644
--- a/libwacom/libwacom-database.c
+++ b/libwacom/libwacom-database.c
@@ -844,14 +844,23 @@ load_tablet_files(WacomDeviceDatabase *db, const char *datadir)
 	DIR *dir;
 	struct dirent *file;
 	bool success = false;
+	GHashTable *keyset = NULL;
 
 	dir = opendir(datadir);
 	if (!dir)
 		return errno == ENOENT; /* non-existing directory is ok */
 
+	/* A set of all matches for duplicate detection. We allow duplicates
+	 * across data directories, but we don't allow for duplicates
+	 * within the same data directory.
+	 */
+	keyset = g_hash_table_new_full (g_str_hash, g_str_equal, free, NULL);
+	if (!keyset)
+		goto out;
+
 	while ((file = readdir(dir))) {
 		WacomDevice *d;
-		const WacomMatch **matches, **match;
+		guint idx = 0;
 
 		if (!is_tablet_file(file))
 			continue;
@@ -860,24 +869,42 @@ load_tablet_files(WacomDeviceDatabase *db, const char *datadir)
 		if (!d)
 			continue;
 
-		matches = libwacom_get_matches(d);
-		if (!matches || !*matches) {
+		if (d->matches->len == 0) {
 			g_critical("Device '%s' has no matches defined\n",
 				   libwacom_get_name(d));
 			goto out;
 		}
 
-		for (match = matches; *match; match++) {
+		/* Note: we may change the array while iterating over it */
+		while (idx < d->matches->len) {
+			WacomMatch *match = g_array_index(d->matches, WacomMatch*, idx);
 			const char *matchstr;
-			matchstr = libwacom_match_get_match_string(*match);
-			/* no duplicate matches allowed */
-			if (g_hash_table_lookup(db->device_ht, matchstr) != NULL) {
+
+			matchstr = libwacom_match_get_match_string(match);
+			/* no duplicate matches allowed within the same
+			 * directory */
+			if (g_hash_table_contains(keyset, matchstr)) {
 				g_critical("Duplicate match of '%s' on device '%s'.",
 					   matchstr, libwacom_get_name(d));
 				goto out;
 			}
-			g_hash_table_insert (db->device_ht, g_strdup (matchstr), d);
+			g_hash_table_add(keyset, g_strdup(matchstr));
+
+			/* We already have an entry for this match in the database,
+			 * that takes precedence. Remove the current match
+			 * from the new tablet - that's fine because we
+			 * haven't exposed the tablet yet.
+			 */
+			if (g_hash_table_lookup(db->device_ht, matchstr)) {
+				libwacom_remove_match(d, match);
+				continue;
+			}
+
+			g_hash_table_insert(db->device_ht,
+					    g_strdup (matchstr),
+					    d);
 			libwacom_ref(d);
+			idx++;
 		}
 		libwacom_unref(d);
 	}
@@ -885,6 +912,8 @@ load_tablet_files(WacomDeviceDatabase *db, const char *datadir)
 	success = true;
 
 out:
+	if (keyset)
+		g_hash_table_destroy(keyset);
 	closedir(dir);
 	return success;
 }
@@ -972,8 +1001,8 @@ LIBWACOM_EXPORT WacomDeviceDatabase *
 libwacom_database_new (void)
 {
 	const char *datadir[] = {
-		DATADIR,
 		ETCDIR,
+		DATADIR,
 	};
 
 	return database_new_for_paths (2, datadir);
diff --git a/libwacom/libwacom.c b/libwacom/libwacom.c
index 107a1e0..d67e05a 100644
--- a/libwacom/libwacom.c
+++ b/libwacom/libwacom.c
@@ -941,6 +941,19 @@ libwacom_add_match(WacomDevice *device, WacomMatch *newmatch)
 	g_array_append_val(device->matches, newmatch);
 }
 
+void
+libwacom_remove_match(WacomDevice *device, WacomMatch *to_remove)
+{
+	for (guint i= 0; i < device->matches->len; i++) {
+		WacomMatch *m = g_array_index(device->matches, WacomMatch*, i);
+		if (streq(m->match, to_remove->match)) {
+			g_array_remove_index(device->matches, i);
+			libwacom_match_unref(to_remove);
+			break;
+		}
+	}
+}
+
 LIBWACOM_EXPORT int
 libwacom_get_vendor_id(const WacomDevice *device)
 {
diff --git a/libwacom/libwacomint.h b/libwacom/libwacomint.h
index 4399ee6..f1e8c62 100644
--- a/libwacom/libwacomint.h
+++ b/libwacom/libwacomint.h
@@ -136,6 +136,7 @@ WacomMatch* libwacom_match_unref(WacomMatch *match);
 
 void libwacom_error_set(WacomError *error, enum WacomErrorCode code, const char *msg, ...);
 void libwacom_add_match(WacomDevice *device, WacomMatch *newmatch);
+void libwacom_remove_match(WacomDevice *device, WacomMatch *newmatch);
 WacomMatch* libwacom_match_new(const char *name, WacomBusType bus,
 			       int vendor_id, int product_id);