[Spice-devel] [spice-gtk] usb-redir: use persistent libusb device on Windows

Yuri Benditovich yuri.benditovich at daynix.com
Mon Feb 11 09:42:03 UTC 2019


Discard unneeded GUDev simulation and use persistent libusb
device pointer in SpiceUsbDevice on Windows as we do on Linux.
This removes significant part of differences between Linux and
Windows code. GUDevClient in win-usb-dev.c now  maintains list
of libusb_device pointers and passes new and removed libusb devices
to usb-device-manager via signal.
Important part of the change is that usb-device-manager on Windows
does not create its own libusb context and uses one created by
win-usb-dev (as libusb device must be enumerated and redirected
with the same libusb context).

Signed-off-by: Yuri Benditovich <yuri.benditovich at daynix.com>
---
 src/usb-device-manager.c | 290 +++++++--------------------------------
 src/win-usb-dev.c        | 254 ++++++++++------------------------
 src/win-usb-dev.h        |  34 +----
 3 files changed, 126 insertions(+), 452 deletions(-)

diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c
index 55b672b..009b15b 100644
--- a/src/usb-device-manager.c
+++ b/src/usb-device-manager.c
@@ -33,7 +33,6 @@
 
 #if defined(G_OS_WIN32)
 #include "win-usb-dev.h"
-#define USE_GUDEV /* win-usb-dev.h provides a fake gudev interface */
 #endif
 
 #include "channel-usbredir-priv.h"
@@ -106,11 +105,10 @@ struct _SpiceUsbDeviceManagerPrivate {
     struct usbredirfilter_rule *redirect_on_connect_rules;
     int auto_conn_filter_rules_count;
     int redirect_on_connect_rules_count;
-#ifdef USE_GUDEV
+#ifdef G_OS_WIN32
     GUdevClient *udev;
-    libusb_device **coldplug_list; /* Avoid needless reprobing during init */
 #else
-    gboolean redirecting; /* Handled by GUdevClient in the gudev case */
+    gboolean redirecting; /* Handled by GUdevClient in Windows */
     libusb_hotplug_callback_handle hp_handle;
 #endif
 #ifdef G_OS_WIN32
@@ -141,11 +139,7 @@ typedef struct _SpiceUsbDeviceInfo {
     guint16 vid;
     guint16 pid;
     gboolean isochronous;
-#ifdef G_OS_WIN32
-    guint8  state;
-#else
     libusb_device *libdev;
-#endif
     gint    ref;
 } SpiceUsbDeviceInfo;
 
@@ -156,13 +150,11 @@ static void channel_destroy(SpiceSession *session, SpiceChannel *channel,
                             gpointer user_data);
 static void channel_event(SpiceChannel *channel, SpiceChannelEvent event,
                           gpointer user_data);
-#ifdef USE_GUDEV
+#ifdef G_OS_WIN32
 static void spice_usb_device_manager_uevent_cb(GUdevClient     *client,
-                                               const gchar     *action,
-                                               GUdevDevice     *udevice,
+                                               libusb_device   *libdev,
+                                               gboolean         add,
                                                gpointer         user_data);
-static void spice_usb_device_manager_add_udev(SpiceUsbDeviceManager  *self,
-                                              GUdevDevice            *udev);
 #else
 static int spice_usb_device_manager_hotplug_cb(libusb_context       *ctx,
                                                libusb_device        *device,
@@ -210,7 +202,7 @@ G_DEFINE_BOXED_TYPE(SpiceUsbDevice, spice_usb_device,
 static void
 _set_redirecting(SpiceUsbDeviceManager *self, gboolean is_redirecting)
 {
-#ifdef USE_GUDEV
+#ifdef G_OS_WIN32
     g_object_set(self->priv->udev, "redirecting", is_redirecting, NULL);
 #else
     self->priv->redirecting = is_redirecting;
@@ -235,7 +227,7 @@ gboolean spice_usb_device_manager_is_redirecting(SpiceUsbDeviceManager *self)
 {
 #ifdef USE_USBREDIR
 
-#ifdef USE_GUDEV
+#ifdef G_OS_WIN32
     gboolean redirecting;
     g_object_get(self->priv->udev, "redirecting", &redirecting, NULL);
     return redirecting;
@@ -286,11 +278,20 @@ static gboolean spice_usb_device_manager_initable_init(GInitable  *initable,
     SpiceUsbDeviceManagerPrivate *priv = self->priv;
     GList *list;
     GList *it;
-    int rc;
-#ifdef USE_GUDEV
-    const gchar *const subsystems[] = {"usb", NULL};
-#endif
 
+    /* Start listening for usb devices plug / unplug */
+#ifdef G_OS_WIN32
+    /* libusb_init called by win-usb-dev */
+    priv->udev = g_udev_client_new(&priv->context);
+    if (priv->udev == NULL) {
+        g_warning("Error initializing GUdevClient");
+        return FALSE;
+    }
+    g_signal_connect(G_OBJECT(priv->udev), "uevent",
+                     G_CALLBACK(spice_usb_device_manager_uevent_cb), self);
+    g_udev_client_report_devices(priv->udev);
+#else
+    int rc;
     /* Initialize libusb */
     rc = libusb_init(&priv->context);
     if (rc < 0) {
@@ -300,33 +301,6 @@ static gboolean spice_usb_device_manager_initable_init(GInitable  *initable,
                     "Error initializing USB support: %s [%i]", desc, rc);
         return FALSE;
     }
-
-#ifdef G_OS_WIN32
-#if LIBUSB_API_VERSION >= 0x01000106
-    libusb_set_option(priv->context, LIBUSB_OPTION_USE_USBDK);
-#endif
-#endif
-
-    /* Start listening for usb devices plug / unplug */
-#ifdef USE_GUDEV
-    priv->udev = g_udev_client_new(subsystems);
-    if (priv->udev == NULL) {
-        g_warning("Error initializing GUdevClient");
-        return FALSE;
-    }
-    g_signal_connect(G_OBJECT(priv->udev), "uevent",
-                     G_CALLBACK(spice_usb_device_manager_uevent_cb), self);
-    /* Do coldplug (detection of already connected devices) */
-    libusb_get_device_list(priv->context, &priv->coldplug_list);
-    list = g_udev_client_query_by_subsystem(priv->udev, "usb");
-    for (it = g_list_first(list); it; it = g_list_next(it)) {
-        spice_usb_device_manager_add_udev(self, it->data);
-        g_object_unref(it->data);
-    }
-    g_list_free(list);
-    libusb_free_device_list(priv->coldplug_list, 1);
-    priv->coldplug_list = NULL;
-#else
     rc = libusb_hotplug_register_callback(priv->context,
         LIBUSB_HOTPLUG_EVENT_DEVICE_ARRIVED | LIBUSB_HOTPLUG_EVENT_DEVICE_LEFT,
         LIBUSB_HOTPLUG_ENUMERATE, LIBUSB_HOTPLUG_MATCH_ANY,
@@ -366,7 +340,7 @@ static void spice_usb_device_manager_dispose(GObject *gobject)
     SpiceUsbDeviceManager *self = SPICE_USB_DEVICE_MANAGER(gobject);
     SpiceUsbDeviceManagerPrivate *priv = self->priv;
 
-#ifndef USE_GUDEV
+#ifndef G_OS_WIN32
     if (priv->hp_handle) {
         spice_usb_device_manager_stop_event_listening(self);
         if (g_atomic_int_get(&priv->event_thread_run)) {
@@ -405,12 +379,15 @@ static void spice_usb_device_manager_finalize(GObject *gobject)
         g_ptr_array_unref(priv->devices);
 
 #ifdef USE_USBREDIR
-#ifdef USE_GUDEV
+#ifdef G_OS_WIN32
     g_clear_object(&priv->udev);
 #endif
     g_return_if_fail(priv->event_thread == NULL);
+#ifndef G_OS_WIN32
+    /* under Windows we reuse existing libusb context */
     if (priv->context)
         libusb_exit(priv->context);
+#endif
     free(priv->auto_conn_filter_rules);
     free(priv->redirect_on_connect_rules);
 #ifdef G_OS_WIN32
@@ -735,29 +712,7 @@ static void spice_usb_device_manager_class_init(SpiceUsbDeviceManagerClass *klas
 #ifdef USE_USBREDIR
 
 /* ------------------------------------------------------------------ */
-/* gudev / libusb Helper functions                                    */
-
-#ifdef USE_GUDEV
-static gboolean spice_usb_device_manager_get_udev_bus_n_address(
-    SpiceUsbDeviceManager *manager, GUdevDevice *udev,
-    int *bus, int *address)
-{
-    const gchar *bus_str, *address_str;
-
-    *bus = *address = 0;
-
-   /* Linux or UsbDk backend on Windows*/
-    bus_str = g_udev_device_get_property(udev, "BUSNUM");
-    address_str = g_udev_device_get_property(udev, "DEVNUM");
-
-    if (bus_str)
-        *bus = atoi(bus_str);
-    if (address_str)
-        *address = atoi(address_str);
-
-    return *bus && *address;
-}
-#endif
+/* libusb Helper functions                                    */
 
 static gboolean spice_usb_device_manager_get_device_descriptor(
     libusb_device *libdev,
@@ -927,17 +882,6 @@ spice_usb_device_manager_device_match(SpiceUsbDeviceManager *self, SpiceUsbDevic
             spice_usb_device_get_devaddr(device) == address);
 }
 
-#ifdef USE_GUDEV
-static gboolean
-spice_usb_device_manager_libdev_match(SpiceUsbDeviceManager *self, libusb_device *libdev,
-                                      const int bus, const int address)
-{
-    /* match functions for Linux/UsbDk -- match by bus.addr */
-    return (libusb_get_bus_number(libdev) == bus &&
-            libusb_get_device_address(libdev) == address);
-}
-#endif
-
 static SpiceUsbDevice*
 spice_usb_device_manager_find_device(SpiceUsbDeviceManager *self,
                                      const int bus, const int address)
@@ -970,6 +914,16 @@ static void spice_usb_device_manager_add_dev(SpiceUsbDeviceManager  *self,
     if (desc.bDeviceClass == LIBUSB_CLASS_HUB)
         return;
 
+    if (spice_usb_device_manager_find_device(self,
+                                    libusb_get_bus_number(libdev),
+                                    libusb_get_device_address(libdev))) {
+        SPICE_DEBUG("device not added %04x:%04x (%p)",
+                    desc.idVendor,
+                    desc.idProduct,
+                    libdev);
+        return;
+    }
+
     device = (SpiceUsbDevice*)spice_usb_device_new(libdev);
     if (!device)
         return;
@@ -994,10 +948,11 @@ static void spice_usb_device_manager_add_dev(SpiceUsbDeviceManager  *self,
                                    spice_usb_device_ref(device));
     }
 
-    SPICE_DEBUG("device added %04x:%04x (%p)",
+    SPICE_DEBUG("device added %04x:%04x (%p, libusb %p)",
                 spice_usb_device_get_vid(device),
                 spice_usb_device_get_pid(device),
-                device);
+                device,
+                libdev);
     g_signal_emit(self, signals[DEVICE_ADDED], 0, device);
 }
 
@@ -1027,80 +982,20 @@ static void spice_usb_device_manager_remove_dev(SpiceUsbDeviceManager *self,
     spice_usb_device_unref(device);
 }
 
-#ifdef USE_GUDEV
-static void spice_usb_device_manager_add_udev(SpiceUsbDeviceManager  *self,
-                                              GUdevDevice            *udev)
-{
-    SpiceUsbDeviceManagerPrivate *priv = self->priv;
-    libusb_device *libdev = NULL, **dev_list = NULL;
-    SpiceUsbDevice *device;
-    const gchar *devtype;
-    int i, bus, address;
-
-    devtype = g_udev_device_get_property(udev, "DEVTYPE");
-    /* Check if this is a usb device (and not an interface) */
-    if (!devtype || strcmp(devtype, "usb_device"))
-        return;
-
-    if (!spice_usb_device_manager_get_udev_bus_n_address(self, udev, &bus, &address)) {
-        g_warning("USB device without bus number or device address");
-        return;
-    }
-
-    device = spice_usb_device_manager_find_device(self, bus, address);
-    if (device) {
-        SPICE_DEBUG("USB device 0x%04x:0x%04x at %d.%d already exists, ignored",
-                    spice_usb_device_get_vid(device),
-                    spice_usb_device_get_pid(device),
-                    spice_usb_device_get_busnum(device),
-                    spice_usb_device_get_devaddr(device));
-        return;
-    }
-
-    if (priv->coldplug_list)
-        dev_list = priv->coldplug_list;
-    else
-        libusb_get_device_list(priv->context, &dev_list);
-
-    for (i = 0; dev_list && dev_list[i]; i++) {
-        if (spice_usb_device_manager_libdev_match(self, dev_list[i], bus, address)) {
-            libdev = dev_list[i];
-            break;
-        }
-    }
-
-    if (libdev)
-        spice_usb_device_manager_add_dev(self, libdev);
-    else
-        g_warning("Could not find USB device to add " DEV_ID_FMT,
-                  (guint) bus, (guint) address);
-
-    if (!priv->coldplug_list)
-        libusb_free_device_list(dev_list, 1);
-}
-
-static void spice_usb_device_manager_remove_udev(SpiceUsbDeviceManager  *self,
-                                                 GUdevDevice            *udev)
-{
-    int bus, address;
-
-    if (!spice_usb_device_manager_get_udev_bus_n_address(self, udev, &bus, &address))
-        return;
-
-    spice_usb_device_manager_remove_dev(self, bus, address);
-}
-
+#ifdef G_OS_WIN32
 static void spice_usb_device_manager_uevent_cb(GUdevClient     *client,
-                                               const gchar     *action,
-                                               GUdevDevice     *udevice,
+                                               libusb_device   *libdev,
+                                               gboolean         add,
                                                gpointer         user_data)
 {
     SpiceUsbDeviceManager *self = SPICE_USB_DEVICE_MANAGER(user_data);
 
-    if (g_str_equal(action, "add"))
-        spice_usb_device_manager_add_udev(self, udevice);
-    else if (g_str_equal (action, "remove"))
-        spice_usb_device_manager_remove_udev(self, udevice);
+    if (add)
+        spice_usb_device_manager_add_dev(self, libdev);
+    else
+        spice_usb_device_manager_remove_dev(self,
+                                    libusb_get_bus_number(libdev),
+                                    libusb_get_device_address(libdev));
 }
 #else
 struct hotplug_idle_cb_args {
@@ -1240,10 +1135,6 @@ static void spice_usb_device_manager_check_redir_on_connect(
             continue;
 
         libdev = spice_usb_device_manager_device_to_libdev(self, device);
-#ifdef G_OS_WIN32
-        if (libdev == NULL)
-            continue;
-#endif
         if (usbredirhost_check_device_filter(
                             priv->redirect_on_connect_rules,
                             priv->redirect_on_connect_rules_count,
@@ -1348,10 +1239,6 @@ GPtrArray* spice_usb_device_manager_get_devices_with_filter(
         if (rules) {
             libusb_device *libdev =
                 spice_usb_device_manager_device_to_libdev(self, device);
-#ifdef G_OS_WIN32
-            if (libdev == NULL)
-                continue;
-#endif
             if (usbredirhost_check_device_filter(rules, count, libdev, 0) != 0)
                 continue;
         }
@@ -1443,24 +1330,6 @@ _spice_usb_device_manager_connect_device_async(SpiceUsbDeviceManager *self,
             continue; /* Skip already used channels */
 
         libdev = spice_usb_device_manager_device_to_libdev(self, device);
-#ifdef G_OS_WIN32
-        if (libdev == NULL) {
-            /* Most likely, the device was plugged out at driver installation
-             * time, and its remove-device event was ignored.
-             * So remove the device now
-             */
-            SPICE_DEBUG("libdev does not exist for %p -- removing", device);
-            spice_usb_device_ref(device);
-            g_ptr_array_remove(priv->devices, device);
-            g_signal_emit(self, signals[DEVICE_REMOVED], 0, device);
-            spice_usb_device_unref(device);
-            g_task_return_new_error(task,
-                                    SPICE_CLIENT_ERROR,
-                                    SPICE_CLIENT_ERROR_FAILED,
-                                    _("Device was not found"));
-            goto done;
-        }
-#endif
         spice_usbredir_channel_connect_device_async(channel,
                                  libdev,
                                  device,
@@ -1732,13 +1601,6 @@ spice_usb_device_manager_can_redirect_device(SpiceUsbDeviceManager  *self,
         libusb_device *libdev;
 
         libdev = spice_usb_device_manager_device_to_libdev(self, device);
-#ifdef G_OS_WIN32
-        if (libdev == NULL) {
-            g_set_error_literal(err, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
-                                _("Some USB devices were not found"));
-            return FALSE;
-        }
-#endif
         filter_ok = (usbredirhost_check_device_filter(
                             guest_filter_rules, guest_filter_rules_count,
                             libdev, 0) == 0);
@@ -1889,9 +1751,7 @@ static SpiceUsbDeviceInfo *spice_usb_device_new(libusb_device *libdev)
     info->pid = pid;
     info->ref = 1;
     info->isochronous = probe_isochronous_endpoint(libdev);
-#ifndef G_OS_WIN32
     info->libdev = libusb_ref_device(libdev);
-#endif
 
     return info;
 }
@@ -2027,14 +1887,11 @@ static void spice_usb_device_unref(SpiceUsbDevice *device)
 
     ref_count_is_0 = g_atomic_int_dec_and_test(&info->ref);
     if (ref_count_is_0) {
-#ifndef G_OS_WIN32
         libusb_unref_device(info->libdev);
-#endif
         g_free(info);
     }
 }
 
-#ifndef G_OS_WIN32 /* Linux -- directly compare libdev */
 static gboolean
 spice_usb_manager_device_equal_libdev(SpiceUsbDeviceManager *manager,
                                       SpiceUsbDevice *device,
@@ -2047,23 +1904,6 @@ spice_usb_manager_device_equal_libdev(SpiceUsbDeviceManager *manager,
 
     return info->libdev == libdev;
 }
-#else /* Windows -- compare vid:pid of device and libdev */
-static gboolean
-spice_usb_manager_device_equal_libdev(SpiceUsbDeviceManager *manager,
-                                      SpiceUsbDevice *device,
-                                      libusb_device  *libdev)
-{
-    int busnum, devaddr;
-
-    if ((device == NULL) || (libdev == NULL))
-        return FALSE;
-
-    busnum = spice_usb_device_get_busnum(device);
-    devaddr = spice_usb_device_get_devaddr(device);
-    return spice_usb_device_manager_libdev_match(manager, libdev,
-                                                 busnum, devaddr);
-}
-#endif
 
 /*
  * Caller must libusb_unref_device the libusb_device returned by this function.
@@ -2073,42 +1913,8 @@ static libusb_device *
 spice_usb_device_manager_device_to_libdev(SpiceUsbDeviceManager *self,
                                           SpiceUsbDevice *device)
 {
-#ifdef G_OS_WIN32
-    /*
-     * On win32 we need to do this the hard and slow way, by asking libusb to
-     * re-enumerate all devices and then finding a matching device.
-     * We cannot cache the libusb_device like we do under Linux since the
-     * driver swap we do under windows invalidates the cached libdev.
-     */
-
-    libusb_device *d, **devlist;
-    int i;
-
-    g_return_val_if_fail(SPICE_IS_USB_DEVICE_MANAGER(self), NULL);
-    g_return_val_if_fail(device != NULL, NULL);
-    g_return_val_if_fail(self->priv != NULL, NULL);
-    g_return_val_if_fail(self->priv->context != NULL, NULL);
-
-    libusb_get_device_list(self->priv->context, &devlist);
-    if (!devlist)
-        return NULL;
-
-    for (i = 0; (d = devlist[i]) != NULL; i++) {
-        if (spice_usb_manager_device_equal_libdev(self, device, d)) {
-            libusb_ref_device(d);
-            break;
-        }
-    }
-
-    libusb_free_device_list(devlist, 1);
-
-    return d;
-
-#else
     /* Simply return a ref to the cached libdev */
     SpiceUsbDeviceInfo *info = (SpiceUsbDeviceInfo *)device;
-
     return libusb_ref_device(info->libdev);
-#endif
 }
 #endif /* USE_USBREDIR */
diff --git a/src/win-usb-dev.c b/src/win-usb-dev.c
index 327976d..ef6804f 100644
--- a/src/win-usb-dev.c
+++ b/src/win-usb-dev.c
@@ -49,31 +49,6 @@ G_DEFINE_TYPE_WITH_CODE(GUdevClient, g_udev_client, G_TYPE_OBJECT,
                         G_ADD_PRIVATE(GUdevClient)
                         G_IMPLEMENT_INTERFACE(G_TYPE_INITABLE, g_udev_client_initable_iface_init))
 
-
-typedef struct _GUdevDeviceInfo GUdevDeviceInfo;
-
-struct _GUdevDeviceInfo {
-    guint16 bus;
-    guint16 addr;
-    guint16 vid;
-    guint16 pid;
-    guint16 class;
-    gchar sclass[4];
-    gchar sbus[4];
-    gchar saddr[4];
-    gchar svid[8];
-    gchar spid[8];
-};
-
-struct _GUdevDevicePrivate
-{
-    /* FixMe: move above fields to this structure and access them directly */
-    GUdevDeviceInfo *udevinfo;
-};
-
-G_DEFINE_TYPE_WITH_PRIVATE(GUdevDevice, g_udev_device, G_TYPE_OBJECT)
-
-
 enum
 {
     UEVENT_SIGNAL,
@@ -83,33 +58,35 @@ enum
 static guint signals[LAST_SIGNAL] = { 0, };
 static GUdevClient *singleton = NULL;
 
-static GUdevDevice *g_udev_device_new(GUdevDeviceInfo *udevinfo);
 static LRESULT CALLBACK wnd_proc(HWND hwnd, UINT message, WPARAM wparam, LPARAM lparam);
-static gboolean get_usb_dev_info(libusb_device *dev, GUdevDeviceInfo *udevinfo);
 
 //uncomment to debug gudev device lists.
-//#define DEBUG_GUDEV_DEVICE_LISTS
+#define DEBUG_GUDEV_DEVICE_LISTS
 
 #ifdef DEBUG_GUDEV_DEVICE_LISTS
 static void g_udev_device_print_list(GList *l, const gchar *msg);
 #else
 static void g_udev_device_print_list(GList *l, const gchar *msg) {}
 #endif
-static void g_udev_device_print(GUdevDevice *udev, const gchar *msg);
+static void g_udev_device_print(libusb_device *dev, const gchar *msg);
 
-static gboolean g_udev_skip_search(GUdevDevice *udev);
+static gboolean g_udev_skip_search(libusb_device *dev);
 
 GQuark g_udev_client_error_quark(void)
 {
     return g_quark_from_static_string("win-gudev-client-error-quark");
 }
 
-GUdevClient *g_udev_client_new(const gchar* const *subsystems)
+GUdevClient *g_udev_client_new(libusb_context **pctx)
 {
-    if (singleton != NULL)
+    if (singleton != NULL) {
+        *pctx = singleton->priv->ctx;
         return g_object_ref(singleton);
-
+    }
     singleton = g_initable_new(G_UDEV_TYPE_CLIENT, NULL, NULL, NULL);
+    if (singleton) {
+        *pctx = singleton->priv->ctx;
+    }
     return singleton;
 }
 
@@ -125,8 +102,6 @@ g_udev_client_list_devices(GUdevClient *self, GList **devs,
     gssize rc;
     libusb_device **lusb_list, **dev;
     GUdevClientPrivate *priv;
-    GUdevDeviceInfo *udevinfo;
-    GUdevDevice *udevice;
     ssize_t n;
 
     g_return_val_if_fail(G_UDEV_IS_CLIENT(self), -1);
@@ -148,14 +123,10 @@ g_udev_client_list_devices(GUdevClient *self, GList **devs,
 
     n = 0;
     for (dev = lusb_list; *dev; dev++) {
-        udevinfo = g_new0(GUdevDeviceInfo, 1);
-        get_usb_dev_info(*dev, udevinfo);
-        udevice = g_udev_device_new(udevinfo);
-        if (g_udev_skip_search(udevice)) {
-            g_object_unref(udevice);
+        if (g_udev_skip_search(*dev)) {
             continue;
         }
-        *devs = g_list_prepend(*devs, udevice);
+        *devs = g_list_prepend(*devs, libusb_ref_device(*dev));
         n++;
     }
     libusb_free_device_list(lusb_list, 1);
@@ -163,11 +134,17 @@ g_udev_client_list_devices(GUdevClient *self, GList **devs,
     return n;
 }
 
+static void unreference_libusb_device(gpointer data)
+{
+    libusb_unref_device((libusb_device *)data);
+}
+
 static void g_udev_client_free_device_list(GList **devs)
 {
     g_return_if_fail(devs != NULL);
     if (*devs) {
-        g_list_free_full(*devs, g_object_unref);
+        /* avoiding casting of imported procedure to GDestroyNotify */
+        g_list_free_full(*devs, unreference_libusb_device);
         *devs = NULL;
     }
 }
@@ -243,11 +220,16 @@ static void g_udev_client_initable_iface_init(GInitableIface *iface)
     iface->init = g_udev_client_initable_init;
 }
 
-GList *g_udev_client_query_by_subsystem(GUdevClient *self, const gchar *subsystem)
+static void report_one_device(gpointer data, gpointer user_data)
+{
+    GUdevClient *self = user_data;
+    g_signal_emit(self, signals[UEVENT_SIGNAL], 0, data, TRUE);
+}
+
+void g_udev_client_report_devices(GUdevClient *self)
 {
     GList *l = g_list_copy(self->priv->udev_list);
-    g_list_foreach(l, (GFunc)g_object_ref, NULL);
-    return l;
+    g_list_foreach(l, report_one_device, self);
 }
 
 static void g_udev_client_init(GUdevClient *self)
@@ -335,11 +317,11 @@ static void g_udev_client_class_init(GUdevClientClass *klass)
                      G_SIGNAL_RUN_FIRST,
                      G_STRUCT_OFFSET(GUdevClientClass, uevent),
                      NULL, NULL,
-                     g_cclosure_user_marshal_VOID__BOXED_BOXED,
+                     g_cclosure_user_marshal_VOID__POINTER_INT,
                      G_TYPE_NONE,
                      2,
-                     G_TYPE_STRING,
-                     G_UDEV_TYPE_DEVICE);
+                     G_TYPE_POINTER,
+                     G_TYPE_INT);
 
     /**
     * GUdevClient::redirecting:
@@ -356,41 +338,22 @@ static void g_udev_client_class_init(GUdevClientClass *klass)
     g_object_class_install_property(gobject_class, PROP_REDIRECTING, pspec);
 }
 
-static gboolean get_usb_dev_info(libusb_device *dev, GUdevDeviceInfo *udevinfo)
+/* Only bus:addr are compared */
+static gint compare_libusb_devices(gconstpointer a, gconstpointer b)
 {
-    struct libusb_device_descriptor desc;
-
-    g_return_val_if_fail(dev, FALSE);
-    g_return_val_if_fail(udevinfo, FALSE);
+    struct {
+        uint8_t bus;
+        uint8_t addr;
+    } a_info, b_info, *ai = &a_info, *bi = &b_info;
 
-    if (libusb_get_device_descriptor(dev, &desc) < 0) {
-        g_warning("cannot get device descriptor %p", dev);
-        return FALSE;
-    }
-
-    udevinfo->bus   = libusb_get_bus_number(dev);
-    udevinfo->addr  = libusb_get_device_address(dev);
-    udevinfo->class = desc.bDeviceClass;
-    udevinfo->vid   = desc.idVendor;
-    udevinfo->pid   = desc.idProduct;
-    g_snprintf(udevinfo->sclass, sizeof(udevinfo->sclass), "%d", udevinfo->class);
-    g_snprintf(udevinfo->sbus,   sizeof(udevinfo->sbus),   "%d", udevinfo->bus);
-    g_snprintf(udevinfo->saddr,  sizeof(udevinfo->saddr),  "%d", udevinfo->addr);
-    g_snprintf(udevinfo->svid,   sizeof(udevinfo->svid),   "%d", udevinfo->vid);
-    g_snprintf(udevinfo->spid,   sizeof(udevinfo->spid),   "%d", udevinfo->pid);
-    return TRUE;
-}
+    ai->bus = libusb_get_bus_number((libusb_device *)a);
+    ai->addr = libusb_get_device_address((libusb_device *)a);
+    bi->bus = libusb_get_bus_number((libusb_device *)b);
+    bi->addr = libusb_get_device_address((libusb_device *)b);
 
-/* Only bus:addr are compared */
-static gint gudev_devices_differ(gconstpointer a, gconstpointer b)
-{
-    GUdevDeviceInfo *ai, *bi;
     gboolean same_bus;
     gboolean same_addr;
 
-    ai = G_UDEV_DEVICE(a)->priv->udevinfo;
-    bi = G_UDEV_DEVICE(b)->priv->udevinfo;
-
     same_bus = (ai->bus == bi->bus);
     same_addr = (ai->addr == bi->addr);
 
@@ -400,15 +363,21 @@ static gint gudev_devices_differ(gconstpointer a, gconstpointer b)
 static void notify_dev_state_change(GUdevClient *self,
                                     GList *old_list,
                                     GList *new_list,
-                                    const gchar *action)
+                                    gboolean add)
 {
     GList *dev;
 
     for (dev = g_list_first(old_list); dev != NULL; dev = g_list_next(dev)) {
-        if (g_list_find_custom(new_list, dev->data, gudev_devices_differ) == NULL) {
+        GList *found = g_list_find_custom(new_list, dev->data, compare_libusb_devices);
+        if (found == NULL) {
             /* Found a device that changed its state */
-            g_udev_device_print(dev->data, action);
-            g_signal_emit(self, signals[UEVENT_SIGNAL], 0, action, dev->data);
+            g_udev_device_print(dev->data, add ? "add" : "remove");
+            g_signal_emit(self, signals[UEVENT_SIGNAL], 0, dev->data, add);
+        } else if (add) {
+            /* keep existing device in new list */
+            GList *temp = found->data;
+            found->data = dev->data;
+            dev->data = temp;
         }
     }
 }
@@ -437,10 +406,10 @@ static void handle_dev_change(GUdevClient *self)
     g_udev_device_print_list(priv->udev_list, "handle_dev_change: previous list:");
 
     /* Unregister devices that are not present anymore */
-    notify_dev_state_change(self, priv->udev_list, now_devs, "remove");
+    notify_dev_state_change(self, priv->udev_list, now_devs, FALSE);
 
     /* Register newly inserted devices */
-    notify_dev_state_change(self, now_devs, priv->udev_list, "add");
+    notify_dev_state_change(self, now_devs, priv->udev_list, TRUE);
 
     /* keep most recent info: free previous list, and keep current list */
     g_udev_client_free_device_list(&priv->udev_list);
@@ -456,84 +425,6 @@ static LRESULT CALLBACK wnd_proc(HWND hwnd, UINT message, WPARAM wparam, LPARAM
     return DefWindowProc(hwnd, message, wparam, lparam);
 }
 
-/*** GUdevDevice ***/
-
-static void g_udev_device_finalize(GObject *object)
-{
-    GUdevDevice *device =  G_UDEV_DEVICE(object);
-
-    g_free(device->priv->udevinfo);
-    if (G_OBJECT_CLASS(g_udev_device_parent_class)->finalize != NULL)
-        (* G_OBJECT_CLASS(g_udev_device_parent_class)->finalize)(object);
-}
-
-static void g_udev_device_class_init(GUdevDeviceClass *klass)
-{
-    GObjectClass *gobject_class = (GObjectClass *) klass;
-
-    gobject_class->finalize = g_udev_device_finalize;
-}
-
-static void g_udev_device_init(GUdevDevice *device)
-{
-    device->priv = g_udev_device_get_instance_private(device);
-}
-
-static GUdevDevice *g_udev_device_new(GUdevDeviceInfo *udevinfo)
-{
-    GUdevDevice *device;
-
-    g_return_val_if_fail(udevinfo != NULL, NULL);
-
-    device =  G_UDEV_DEVICE(g_object_new(G_UDEV_TYPE_DEVICE, NULL));
-    device->priv->udevinfo = udevinfo;
-    return device;
-}
-
-const gchar *g_udev_device_get_property(GUdevDevice *udev, const gchar *property)
-{
-    GUdevDeviceInfo* udevinfo;
-
-    g_return_val_if_fail(G_UDEV_DEVICE(udev), NULL);
-    g_return_val_if_fail(property != NULL, NULL);
-
-    udevinfo = udev->priv->udevinfo;
-    g_return_val_if_fail(udevinfo != NULL, NULL);
-
-    if (g_strcmp0(property, "BUSNUM") == 0) {
-        return udevinfo->sbus;
-    } else if (g_strcmp0(property, "DEVNUM") == 0) {
-        return udevinfo->saddr;
-    } else if (g_strcmp0(property, "DEVTYPE") == 0) {
-        return "usb_device";
-    } else if (g_strcmp0(property, "VID") == 0) {
-        return udevinfo->svid;
-    } else if (g_strcmp0(property, "PID") == 0) {
-        return udevinfo->spid;
-    }
-
-    g_warn_if_reached();
-    return NULL;
-}
-
-const gchar *g_udev_device_get_sysfs_attr(GUdevDevice *udev, const gchar *attr)
-{
-    GUdevDeviceInfo* udevinfo;
-
-    g_return_val_if_fail(G_UDEV_DEVICE(udev), NULL);
-    g_return_val_if_fail(attr != NULL, NULL);
-
-    udevinfo = udev->priv->udevinfo;
-    g_return_val_if_fail(udevinfo != NULL, NULL);
-
-
-    if (g_strcmp0(attr, "bDeviceClass") == 0) {
-        return udevinfo->sclass;
-    }
-    g_warn_if_reached();
-    return NULL;
-}
-
 #ifdef DEBUG_GUDEV_DEVICE_LISTS
 static void g_udev_device_print_list(GList *l, const gchar *msg)
 {
@@ -545,35 +436,40 @@ static void g_udev_device_print_list(GList *l, const gchar *msg)
 }
 #endif
 
-static void g_udev_device_print(GUdevDevice *udev, const gchar *msg)
+static void g_udev_device_print(libusb_device *dev, const gchar *msg)
 {
-    GUdevDeviceInfo* udevinfo;
-
-    g_return_if_fail(G_UDEV_DEVICE(udev));
-
-    udevinfo = udev->priv->udevinfo;
-    g_return_if_fail(udevinfo != NULL);
+    struct libusb_device_descriptor desc;
+    uint8_t bus, addr;
+    g_return_if_fail(dev);
+    if (libusb_get_device_descriptor(dev, &desc) < 0) {
+        return;
+    }
+    addr = libusb_get_device_address(dev);
+    bus  = libusb_get_bus_number(dev);
 
     SPICE_DEBUG("%s: %d.%d 0x%04x:0x%04x class %d", msg,
-                udevinfo->bus, udevinfo->addr,
-                udevinfo->vid, udevinfo->pid, udevinfo->class);
+                bus, addr,
+                desc.idVendor, desc.idProduct, desc.bDeviceClass);
 }
 
-static gboolean g_udev_skip_search(GUdevDevice *udev)
+static gboolean g_udev_skip_search(libusb_device *dev)
 {
-    GUdevDeviceInfo* udevinfo;
     gboolean skip;
+    struct libusb_device_descriptor desc;
+    uint8_t addr;
 
-    g_return_val_if_fail(G_UDEV_DEVICE(udev), FALSE);
+    g_return_val_if_fail(dev, TRUE);
 
-    udevinfo = udev->priv->udevinfo;
-    g_return_val_if_fail(udevinfo != NULL, FALSE);
+    if (libusb_get_device_descriptor(dev, &desc) < 0) {
+        return TRUE;
+    }
+    addr = libusb_get_device_address(dev);
 
-    skip = ((udevinfo->addr == 0xff) ||  /* root hub (HCD) */
+    skip = ((addr == 0xff) ||  /* root hub (HCD) */
 #if defined(LIBUSBX_API_VERSION) && (LIBUSBX_API_VERSION >= 0x010000FF)
-            (udevinfo->addr == 1) || /* root hub addr for libusbx >= 1.0.13 */
+            (addr == 1) || /* root hub addr for libusbx >= 1.0.13 */
 #endif
-            (udevinfo->class == LIBUSB_CLASS_HUB) || /* hub*/
-            (udevinfo->addr == 0)); /* bad address */
+            (desc.bDeviceClass == LIBUSB_CLASS_HUB) || /* hub*/
+            (addr == 0)); /* bad address */
     return skip;
 }
diff --git a/src/win-usb-dev.h b/src/win-usb-dev.h
index 7f40197..9a60956 100644
--- a/src/win-usb-dev.h
+++ b/src/win-usb-dev.h
@@ -26,30 +26,6 @@
 
 G_BEGIN_DECLS
 
-/* GUdevDevice */
-
-#define G_UDEV_TYPE_DEVICE         (g_udev_device_get_type())
-#define G_UDEV_DEVICE(o)           (G_TYPE_CHECK_INSTANCE_CAST((o), G_UDEV_TYPE_DEVICE, GUdevDevice))
-#define G_UDEV_DEVICE_CLASS(k)     (G_TYPE_CHECK_CLASS_CAST((k), G_UDEV_TYPE_DEVICE, GUdevDeviceClass))
-#define G_UDEV_IS_DEVICE(o)        (G_TYPE_CHECK_INSTANCE_TYPE ((o), G_UDEV_TYPE_DEVICE))
-#define G_UDEV_IS_DEVICE_CLASS(k)  (G_TYPE_CHECK_CLASS_TYPE((k), G_UDEV_TYPE_DEVICE))
-#define G_UDEV_DEVICE_GET_CLASS(o) (G_TYPE_INSTANCE_GET_CLASS((o), G_UDEV_TYPE_DEVICE, GUdevDeviceClass))
-
-typedef struct _GUdevDevice GUdevDevice;
-typedef struct _GUdevDeviceClass GUdevDeviceClass;
-typedef struct _GUdevDevicePrivate GUdevDevicePrivate;
-
-struct _GUdevDevice
-{
-  GObject parent;
-  GUdevDevicePrivate *priv;
-};
-
-struct _GUdevDeviceClass
-{
-  GObjectClass parent_class;
-};
-
 /* GUdevClient */
 
 #define G_UDEV_TYPE_CLIENT         (g_udev_client_get_type())
@@ -75,16 +51,12 @@ struct _GUdevClientClass
     GObjectClass parent_class;
 
     /* signals */
-    void (*uevent)(GUdevClient *client, const gchar *action, GUdevDevice  *device);
+    void (*uevent)(GUdevClient *client, libusb_device *libdev, gboolean add);
 };
 
 GType g_udev_client_get_type(void) G_GNUC_CONST;
-GUdevClient *g_udev_client_new(const gchar* const *subsystems);
-GList *g_udev_client_query_by_subsystem(GUdevClient *client, const gchar *subsystem);
-
-GType g_udev_device_get_type(void) G_GNUC_CONST;
-const gchar *g_udev_device_get_property(GUdevDevice *udev, const gchar *property);
-const gchar *g_udev_device_get_sysfs_attr(GUdevDevice *udev, const gchar *attr);
+GUdevClient *g_udev_client_new(libusb_context **pctx);
+void g_udev_client_report_devices(GUdevClient *client);
 
 GQuark g_udev_client_error_quark(void);
 #define G_UDEV_CLIENT_ERROR g_udev_client_error_quark()
-- 
2.17.1



More information about the Spice-devel mailing list