ply-device-manager: push udev_device_get_devnode call up into on_udev_event

on_udev_event calls either on_drm_udev_add_or_change; or
free_devices_for_udev_device. Both of these functions call
udev_device_get_devnode and are a no-op if that returns NULL.

Cleanup things by directly calling udev_device_get_devnode from
on_udev_event and exit eary if the udev_device does not have
an associated devnode.

With the udev_device_get_devnode call handled by on_udev_event,
all that is left of free_devices_for_udev_device is a single
line calling free_devices_from_device_path. So drop
free_devices_for_udev_device and directly call
free_devices_from_device_path from on_udev_event.

Note this also fixes a theoretical udev_device reference leak
in the action == NULL early-exit path. In practice action never
is NULL, so this was not really a problem. But in the refactored
code we now also do the early-exit on dev_path == NULL which does
actually happen.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>

1 files changed, 10 insertions, 23 deletions

diff --git a/src/libply-splash-core/ply-device-manager.c b/src/libply-splash-core/ply-device-manager.c
index f65d7317..9e6fb97c 100644
--- a/src/libply-splash-core/ply-device-manager.c
+++ b/src/libply-splash-core/ply-device-manager.c
@@ -308,18 +308,6 @@ create_devices_for_udev_device (ply_device_manager_t *manager,
         return created;
 }
 
-static void
-free_devices_for_udev_device (ply_device_manager_t *manager,
-                              struct udev_device   *device)
-{
-        const char *device_path;
-
-        device_path = udev_device_get_devnode (device);
-
-        if (device_path != NULL)
-                free_devices_from_device_path (manager, device_path, true);
-}
-
 static bool
 create_devices_for_subsystem (ply_device_manager_t *manager,
                               const char           *subsystem)
@@ -385,15 +373,12 @@ create_devices_for_subsystem (ply_device_manager_t *manager,
 static void
 on_drm_udev_add_or_change (ply_device_manager_t *manager,
                            const char           *action,
+                           const char           *device_path,
                            struct udev_device   *device)
 {
-        const char *device_path = udev_device_get_devnode (device);
         ply_renderer_t *renderer;
         bool changed;
 
-        if (device_path == NULL)
-                return;
-
         renderer = ply_hashtable_lookup (manager->renderers, (void *) device_path);
         if (renderer == NULL) {
                 /* We also try to create the renderer again on change events,
@@ -419,18 +404,20 @@ static void
 on_udev_event (ply_device_manager_t *manager)
 {
         struct udev_device *device;
-        const char *action;
+        const char *action, *device_path;
 
         device = udev_monitor_receive_device (manager->udev_monitor);
         if (device == NULL)
                 return;
 
         action = udev_device_get_action (device);
-
-        ply_trace ("got %s event for device %s", action, udev_device_get_sysname (device));
-
-        if (action == NULL)
+        device_path = udev_device_get_devnode (device);
+        if (action == NULL || device_path == NULL) {
+                udev_device_unref (device);
                 return;
+        }
+
+        ply_trace ("got %s event for device %s", action, device_path);
 
         if (strcmp (action, "add") == 0 || strcmp (action, "change") == 0) {
                 const char *subsystem;
@@ -441,12 +428,12 @@ on_udev_event (ply_device_manager_t *manager)
                         if (manager->local_console_managed && manager->local_console_is_text)
                                 ply_trace ("ignoring since we're already using text splash for local console");
                         else
-                                on_drm_udev_add_or_change (manager, action, device);
+                                on_drm_udev_add_or_change (manager, action, device_path, device);
                 } else {
                         ply_trace ("ignoring since we only handle subsystem %s devices after timeout", subsystem);
                 }
         } else if (strcmp (action, "remove") == 0) {
-                free_devices_for_udev_device (manager, device);
+                free_devices_from_device_path (manager, device_path, true);
         }
 
         udev_device_unref (device);