Skip to content

Commit

Permalink
Assume .tablet files shadow any ones with the same name
Browse files Browse the repository at this point in the history
Commit 2dc213c introduced a duplicate resolution to fix #379 so
that we could have a tablet file in /etc/libwacom that had a DeviceMatch
that partially overlapped with a DeviceMatch in /usr/share/libwacom,
e.g. in this setup:
  DeviceMatch=usb|1234|abcd;bluetooth|1234|abcd
  DeviceMatch=usb|1234|abcd

This triggered a bug: during the duplicate resolution the device match
was removed from the currently parsed file, leading to the file having
no device matches which triggered a g_warn_if_reached().

This approach is too complicated and likely unnecessary, use a more
conventional approach where a named file hide the same file in any
other datadir, i.e. /etc/libwacom/foo-bar.tablet will hide
/usr/share/libwacom/foo-bar.tablet, the latter of which will not be
parsed at all.

This corresponds to how e.g. system handles service files and many other
components in the stack work with a similar approach.

Reverts commit 2dc213c ("libwacom: allow for duplicates across data directories")
  • Loading branch information
whot committed Nov 8, 2024
1 parent a6d7b88 commit cec5366
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 35 deletions.
8 changes: 4 additions & 4 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -183,19 +183,19 @@ jobs:
# 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/wacom-cintiq-27hd.tablet | sudo tee /etc/libwacom/modified-cintiq.tablet
sed -e 's/Pro L/Pro L MODIFIED/' -e 's/usb|056a|0358;//' data/wacom-intuos-pro-2-l.tablet | sudo tee /etc/libwacom/modified-intuos.tablet
sed -e 's/27QHD/27QHD MODIFIED/' data/wacom-cintiq-27hd.tablet | sudo tee /etc/libwacom/wacom-cintiq-27hd.tablet
sed -e 's/Pro L/Pro L MODIFIED/' -e 's/usb|056a|0358;//' data/wacom-intuos-pro-2-l.tablet | sudo tee /etc/libwacom/wacom-intuos-pro-2-l.tablet
- name: list all devices for debugging
run: libwacom-list-devices --format=yaml

# We expect the modified tablets to be listed
# We expect the remaining match for a modified tablet to be listed
# We expect the remaining match for a modified tablet to not be listed
# We expect the overridden match *not* to be listed
- name: check for the expected devices to be present (or not present)
run: |
test "$(libwacom-list-devices --format=yaml | yq -r '.devices[] | select(.bus == "usb") | select(.vid == "0x056a") | select(.pid == "0x032a") | .name')" == "Wacom Cintiq 27QHD MODIFIED"
test "$(libwacom-list-devices --format=yaml | yq -r '.devices[] | select(.bus == "bluetooth") | select(.vid == "0x056a") | select(.pid == "0x0361") | .name')" == "Wacom Intuos Pro L MODIFIED"
test $(libwacom-list-devices --format=yaml | yq -r '.devices[] | select(.bus == "usb") | select(.vid == "0x056a") | select(.pid == "0x0358") | .name' | wc -l) -eq 1
test $(libwacom-list-devices --format=yaml | yq -r '.devices[] | select(.bus == "usb") | select(.vid == "0x056a") | select(.pid == "0x0358") | .name' | wc -l) -eq 0
test $(libwacom-list-devices --format=yaml | yq -r '.devices[] | select(.bus == "usb") | select(.vid == "0x056a") | select(.pid == "0x032a") | .name' | wc -l) -eq 1
####
Expand Down
48 changes: 17 additions & 31 deletions libwacom/libwacom-database.c
Original file line number Diff line number Diff line change
Expand Up @@ -1017,32 +1017,28 @@ is_stylus_file(const struct dirent *entry)
}

static bool
load_tablet_files(WacomDeviceDatabase *db, const char *datadir)
load_tablet_files(WacomDeviceDatabase *db, GHashTable *parsed_filenames, 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;
guint idx = 0;

if (!is_tablet_file(file))
continue;

if (g_hash_table_lookup(parsed_filenames, file->d_name))
continue;

g_hash_table_add(parsed_filenames, g_strdup(file->d_name));

d = libwacom_parse_tablet_keyfile(db, datadir, file->d_name);
if (!d)
continue;
Expand All @@ -1059,28 +1055,13 @@ load_tablet_files(WacomDeviceDatabase *db, const char *datadir)
const char *matchstr;

matchstr = libwacom_match_get_match_string(match);
/* no duplicate matches allowed within the same
* directory */
if (g_hash_table_contains(keyset, matchstr)) {
/* 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));
goto out;
}
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);
g_hash_table_insert(db->device_ht, g_strdup (matchstr), d);
libwacom_ref(d);
idx++;
}
Expand All @@ -1090,8 +1071,6 @@ load_tablet_files(WacomDeviceDatabase *db, const char *datadir)
success = true;

out:
if (keyset)
g_hash_table_destroy(keyset);
closedir(dir);
return success;
}
Expand Down Expand Up @@ -1146,6 +1125,11 @@ database_new_for_paths (char * const *datadirs)
{
WacomDeviceDatabase *db;
char * const *datadir;
GHashTable *parsed_filenames;

parsed_filenames = g_hash_table_new_full (g_str_hash, g_str_equal, free, NULL);
if (!parsed_filenames)
return NULL;

db = g_new0 (WacomDeviceDatabase, 1);
db->device_ht = g_hash_table_new_full (g_str_hash,
Expand All @@ -1163,10 +1147,12 @@ database_new_for_paths (char * const *datadirs)
}

for (datadir = datadirs; *datadir; datadir++) {
if (!load_tablet_files(db, *datadir))
if (!load_tablet_files(db, parsed_filenames, *datadir))
goto error;
}

g_hash_table_unref(parsed_filenames);

/* If we couldn't load _anything_ then something's wrong */
if (g_hash_table_size (db->stylus_ht) == 0 ||
g_hash_table_size (db->device_ht) == 0) {
Expand Down

0 comments on commit cec5366

Please sign in to comment.