GNOME Bugzilla – Bug 707901
Minimal clean up of the evdev backend
Last modified: 2021-06-10 11:32:10 UTC
I'd rather have somebody else test these patches, given that I can't currently test the evdev/kms backend.
Created attachment 254658 [details] [review] evdev: Cache the regexp Instead of recreating it for every new device, we can cache the GRegex and reuse it.
Created attachment 254659 [details] [review] evdev: Clean up debug and error messages
Created attachment 254660 [details] [review] evdev: Minimal coding style fixes Do not use g_[s]list_next(), and do an explicit NULL check instead of relying on the truthy value of a non-NULL pointer.
Created attachment 254661 [details] [review] evdev: Use a hash table of devices for faster look up Instead of iterating over the list of devices.
Review of attachment 254661 [details] [review]: There's some unrelated whitespace change in this patch, you may want to split it out. ::: clutter/evdev/clutter-device-manager-evdev.c @@ +915,3 @@ + sysfs_path = _clutter_input_device_evdev_get_device_path (device_evdev); + g_hash_table_insert (priv->devices_by_path, (gpointer) sysfs_path, device); The core keyboard/pointer won't have a sysfs path (since they're virtual devices), so you must check it. Also, it might be good to note that devices_by_path only includes slave devices (while devices includes everything) @@ +944,3 @@ /* Remove the device */ + sysfs_path = _clutter_input_device_evdev_get_device_path (device_evdev); + g_hash_table_remove (priv->devices_by_path, (gpointer) sysfs_path); Same here, need to check if sysfs_path is null.
Created attachment 254681 [details] [review] evdev: fix a crash when reclaiming devices That was not how you iterate a list!
Review of attachment 254658 [details] [review]: Yes
Review of attachment 254659 [details] [review]: Ok
Review of attachment 254660 [details] [review]: There is more g_slist_next() after this patch, you may want to remove those too.
Review of attachment 254681 [details] [review]: obviously correct.
Comment on attachment 254681 [details] [review] evdev: fix a crash when reclaiming devices Attachment 254681 [details] pushed as b29115e - evdev: fix a crash when reclaiming devices
Attachment 254658 [details] pushed as 9eb479a - evdev: Cache the regexp Attachment 254659 [details] pushed as a1d29ab - evdev: Clean up debug and error messages
GNOME is going to shut down bugzilla.gnome.org in favor of gitlab.gnome.org. As part of that, we are mass-closing older open tickets in bugzilla.gnome.org which have not seen updates for a longer time (resources are unfortunately quite limited so not every ticket can get handled). If you can still reproduce the situation described in this ticket in a recent and supported software version of clutter, then please follow https://wiki.gnome.org/GettingInTouch/BugReportingGuidelines and create a ticket at https://gitlab.gnome.org/GNOME/clutter/-/issues/ Thank you for your understanding and your help.