[PATCH v3 0/1] MR1810: winebus.sys: Fix the build without HAVE_SYS_INOTIFY_H

[Gerald Pfeifer (＠gerald)]

find_device_from_devnode was guarded by HAVE_SYS_INOTIFY_H, alas its
use in process_monitor_event was not, so linking failed.

On the way reduce the scope of a variable (which makes sense per se
and avoids introducing a compiler warning with this change).

--
  v3: winebus.sys: Fix the build without HAVE_SYS_INOTIFY_H
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1810

[PATCH v3 1/1] winebus.sys: Fix the build without HAVE_SYS_INOTIFY_H

[Gerald Pfeifer]

From: Gerald Pfeifer <gerald(a)pfeifer.com>

find_device_from_devnode was unnecessarily guarded by HAVE_SYS_INOTIFY_H,
alas its use in process_monitor_event was not, so linking failed. Simply
make find_device_from_devnode generally available.

On the way reduce the scope of a variable.
---
 dlls/winebus.sys/bus_udev.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/dlls/winebus.sys/bus_udev.c b/dlls/winebus.sys/bus_udev.c
index 71b9ef84530..602591e6bc8 100644
--- a/dlls/winebus.sys/bus_udev.c
+++ b/dlls/winebus.sys/bus_udev.c
@@ -260,6 +260,16 @@ static struct base_device *find_device_from_fd(int fd)
     return NULL;
 }
 
+static struct base_device *find_device_from_devnode(const char *path)
+{
+    struct base_device *impl;
+
+    LIST_FOR_EACH_ENTRY(impl, &device_list, struct base_device, unix_device.entry)
+        if (!strcmp(impl->devnode, path)) return impl;
+
+    return NULL;
+}
+
 static void hidraw_device_destroy(struct unix_device *iface)
 {
     struct hidraw_device *impl = hidraw_impl_from_unix_device(iface);
@@ -1546,16 +1556,6 @@ static int create_inotify(void)
     return fd;
 }
 
-static struct base_device *find_device_from_devnode(const char *path)
-{
-    struct base_device *impl;
-
-    LIST_FOR_EACH_ENTRY(impl, &device_list, struct base_device, unix_device.entry)
-        if (!strcmp(impl->devnode, path)) return impl;
-
-    return NULL;
-}
-
 static void maybe_remove_devnode(const char *base, const char *dir)
 {
     struct base_device *impl;
@@ -1714,7 +1714,6 @@ error:
 
 static void process_monitor_event(struct udev_monitor *monitor)
 {
-    struct base_device *impl;
     struct udev_device *dev;
     const char *action;
 
@@ -1735,6 +1734,8 @@ static void process_monitor_event(struct udev_monitor *monitor)
         udev_add_device(dev, -1);
     else
     {
+        struct base_device *impl;
+
         impl = find_device_from_devnode(udev_device_get_devnode(dev));
         if (impl) bus_event_queue_device_removed(&event_queue, &impl->unix_device);
         else WARN("failed to find device for udev device %p\n", dev);
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/1810

[Rémi Bernon]

Rémi Bernon (@rbernon) commented about dlls/winebus.sys/bus_udev.c:
> udev_add_device(dev, -1);
>      else
>      {
> +        struct base_device *impl;
> +

Could you remove this change? I don't think it's necessary.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1810#note_19757

[Gerald Pfeifer (＠gerald)]

On Fri Dec 16 20:35:03 2022 +0000, Rémi Bernon wrote:
> Could you remove this change? I don't think it's necessary.
> Yes, you're right: it was only necessary to avoid warnings with an earlier version of the patch.

However, isn't it better to have variables at smaller scope? Not for the compiler, which will generate identical code, but for the human brain?

I'll make the change if you wish, just wondering.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1810#note_19759

[Rémi Bernon]

On Fri Dec 16 20:58:35 2022 +0000, Gerald Pfeifer wrote:
> Yes, you're right: it was only necessary to avoid warnings with an
> earlier version of the patch.
> However, isn't it better to have variables at smaller scope? Not for the
> compiler, which will generate identical code, but for the human brain?
> I'll make the change if you wish, just wondering.
> I don't have a strong opinion, although I tend to prefer having variables declared in one place, especially for small functions like this. In any case it's an unrelated and unnecessary change here.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1810#note_19769