[PATCH v2 0/3] MR5385: winebus: ignore udev events of devices which do not have devnodes
Udev monitor monitors the whole input subsystem, but not all devices in the input subsystem have devnodes associated to them. This MR makes the event processing ignore such devices. All device handling assumes devices have devnodes, so here we just simply ignore all devices which do not have one. They are irrelevant. Previously, udev bus thread aborted when an event for a device without a devnode was processed: ``` 10111.330:0068:0084:trace:hid:process_monitor_event Received action "remove" for udev device (null) 10111.330:0068:0084:warn:hid:bus_main_thread L"UDEV" bus wait returned status 0xc0000005 ``` Just plugging in and out a normal mouse was enough cause this. This was because root input devices (which do not have devnodes) were handled too and `find_device_from_devnode()` choked on NULL argument. -- v2: winebus: group local variable declarations https://gitlab.winehq.org/wine/wine/-/merge_requests/5385
From: Tuomas Räsänen <tuomas.rasanen(a)opinsys.fi> --- dlls/winebus.sys/bus_udev.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/dlls/winebus.sys/bus_udev.c b/dlls/winebus.sys/bus_udev.c index 419de2e7af3..228d9d8316d 100644 --- a/dlls/winebus.sys/bus_udev.c +++ b/dlls/winebus.sys/bus_udev.c @@ -264,6 +264,12 @@ static struct base_device *find_device_from_devnode(const char *path) { struct base_device *impl; + if (!path) + { + FIXME("called find_device_from_devnode(NULL)\n"); + return NULL; + } + LIST_FOR_EACH_ENTRY(impl, &device_list, struct base_device, unix_device.entry) if (!strcmp(impl->devnode, path)) return impl; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5385
From: Tuomas Räsänen <tuomas.rasanen(a)opinsys.fi> Udev monitor monitors the whole input subsystem, but not all devices in the input subsystem have devnodes associated to them. This commit makes the event processing ignore such devices. All device handling assumes devices have devnodes, so here we just simply ignore all devices which do not have one. Previously, udev bus thread aborted when an event for a device without a devnode was processed: 10111.330:0068:0084:trace:hid:process_monitor_event Received action "remove" for udev device (null) 10111.330:0068:0084:warn:hid:bus_main_thread L"UDEV" bus wait returned status 0xc0000005 Just plugging in and out a normal mouse was enough cause this. This was because root input devices (which do not have devnodes) were handled too and find_device_from_devnode() choked on NULL argument. --- dlls/winebus.sys/bus_udev.c | 34 +++++++++++++++++++++++++++++----- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/dlls/winebus.sys/bus_udev.c b/dlls/winebus.sys/bus_udev.c index 228d9d8316d..48565ea4d6e 100644 --- a/dlls/winebus.sys/bus_udev.c +++ b/dlls/winebus.sys/bus_udev.c @@ -1717,6 +1717,8 @@ static void process_monitor_event(struct udev_monitor *monitor) struct base_device *impl; struct udev_device *dev; const char *action; + const char *devnode; + const char *syspath; dev = udev_monitor_receive_device(monitor); if (!dev) @@ -1726,20 +1728,42 @@ static void process_monitor_event(struct udev_monitor *monitor) } action = udev_device_get_action(dev); - TRACE("Received action %s for udev device %s\n", debugstr_a(action), - debugstr_a(udev_device_get_devnode(dev))); + syspath = udev_device_get_syspath(dev); + devnode = udev_device_get_devnode(dev); + TRACE("Received action %s for udev device %s (%p) devnode %s\n", + debugstr_a(action), debugstr_a(syspath), dev, debugstr_a(devnode)); + + if (!syspath) + { + FIXME("udev device %p does not have syspath!\n", dev); + goto out; + } if (!action) - WARN("No action received\n"); - else if (strcmp(action, "remove")) + { + WARN("event for udev device %s does not have any action!\n", syspath); + goto out; + } + + if (!devnode) + { + // Pretty normal case, not all devices have associated + // devnodes. For example root input devices do not, but + // related/child mouse and event devices do. + TRACE("udev device %s does not have devnode, ignoring\n", syspath); + goto out; + } + + if (strcmp(action, "remove")) udev_add_device(dev, -1); else { - impl = find_device_from_devnode(udev_device_get_devnode(dev)); + impl = find_device_from_devnode(devnode); if (impl) bus_event_queue_device_removed(&event_queue, &impl->unix_device); else WARN("failed to find device for udev device %p\n", dev); } +out: udev_device_unref(dev); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5385
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winebus.sys/bus_udev.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/dlls/winebus.sys/bus_udev.c b/dlls/winebus.sys/bus_udev.c index 48565ea4d6e..7a5cb3d20c6 100644 --- a/dlls/winebus.sys/bus_udev.c +++ b/dlls/winebus.sys/bus_udev.c @@ -1716,9 +1716,7 @@ static void process_monitor_event(struct udev_monitor *monitor) { struct base_device *impl; struct udev_device *dev; - const char *action; - const char *devnode; - const char *syspath; + const char *action, devnode, syspath; dev = udev_monitor_receive_device(monitor); if (!dev) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5385
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=144382 Your paranoid android. === debian11 (build log) === ../wine/dlls/winebus.sys/bus_udev.c:1729:13: error: assignment of read-only variable ���syspath��� ../wine/dlls/winebus.sys/bus_udev.c:1730:13: error: assignment of read-only variable ���devnode��� Task: The win32 Wine build failed === debian11b (build log) === ../wine/dlls/winebus.sys/bus_udev.c:1729:13: error: assignment of read-only variable ���syspath��� ../wine/dlls/winebus.sys/bus_udev.c:1730:13: error: assignment of read-only variable ���devnode��� Task: The wow64 Wine build failed
participants (3)
-
Marvin -
Rémi Bernon -
Tuomas Räsänen