On Sun, Mar 10, 2019 at 04:46:09PM +0200, Yuri Benditovich wrote: > Change internal device list to maintain libusb devices > instead of GUdevDevice objects. Create temporary > GUdevDevice object only for indication to usb-dev-manager. > > Signed-off-by: Yuri Benditovich <yuri.benditov...@daynix.com> > --- > src/win-usb-dev.c | 80 ++++++++++++++++++++++++++++++----------------- > 1 file changed, 51 insertions(+), 29 deletions(-) > > diff --git a/src/win-usb-dev.c b/src/win-usb-dev.c > index 1ab704d..5d878ea 100644 > --- a/src/win-usb-dev.c > +++ b/src/win-usb-dev.c > @@ -95,7 +95,7 @@ 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(libusb_device *dev); > > @@ -129,8 +129,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); > @@ -155,10 +153,7 @@ g_udev_client_list_devices(GUdevClient *self, GList > **devs, > if (g_udev_skip_search(*dev)) { > continue; > } > - udevinfo = g_new0(GUdevDeviceInfo, 1); > - get_usb_dev_info(*dev, udevinfo); > - udevice = g_udev_device_new(udevinfo); > - *devs = g_list_prepend(*devs, udevice); > + *devs = g_list_prepend(*devs, libusb_ref_device(*dev)); > n++; > } > libusb_free_device_list(lusb_list, 1); > @@ -166,11 +161,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);
Since all unreference_libusb_device is doing is blindly casting a void * to libusb_device *, I'd just use a cast to GDestroyNotify here rather than introduce an intermediate method. If you prefer to keep that intermediate helper, please remove the commit which I don't think is needed. > *devs = NULL; > } > } > @@ -246,9 +247,22 @@ static void > g_udev_client_initable_iface_init(GInitableIface *iface) > iface->init = g_udev_client_initable_init; > } > > +static void g_udev_notify_device(GUdevClient *self, libusb_device *dev, int > add) > +{ > + GUdevDeviceInfo *udevinfo; > + GUdevDevice *udevice; > + udevinfo = g_new0(GUdevDeviceInfo, 1); > + if (get_usb_dev_info(dev, udevinfo)) { > + udevice = g_udev_device_new(udevinfo); > + g_signal_emit(self, signals[UEVENT_SIGNAL], 0, udevice, add); > + } else { > + g_free(udevinfo); > + } > +} > + > static void report_one_device(gpointer data, gpointer self) > { > - g_signal_emit(self, signals[UEVENT_SIGNAL], 0, data, TRUE); > + g_udev_notify_device(self, data, TRUE); > } > > void g_udev_client_report_devices(GUdevClient *self) > @@ -388,18 +402,20 @@ static gboolean get_usb_dev_info(libusb_device *dev, > GUdevDeviceInfo *udevinfo) > } > > /* comparing bus:addr and vid:pid */ > -static gint gudev_devices_differ(gconstpointer a, gconstpointer b) > +static gint compare_libusb_devices(gconstpointer a, gconstpointer b) > { > - GUdevDeviceInfo *ai, *bi; > + libusb_device *a_dev = (libusb_device *)a; > + libusb_device *b_dev = (libusb_device *)b; > + struct libusb_device_descriptor a_desc, b_desc; > gboolean same_bus, same_addr, same_vid, same_pid; > > - ai = G_UDEV_DEVICE(a)->priv->udevinfo; > - bi = G_UDEV_DEVICE(b)->priv->udevinfo; > + libusb_get_device_descriptor(a_dev, &a_desc); > + libusb_get_device_descriptor(b_dev, &b_desc); > > - same_bus = (ai->bus == bi->bus); > - same_addr = (ai->addr == bi->addr); > - same_vid = (ai->vid == bi->vid); > - same_pid = (ai->pid == bi->pid); > + same_bus = libusb_get_bus_number(a_dev) == libusb_get_bus_number(b_dev); > + same_addr = libusb_get_device_address(a_dev) == > libusb_get_device_address(b_dev); > + same_vid = (a_desc.idVendor == b_desc.idVendor); > + same_pid = (a_desc.idProduct == b_desc.idProduct); > > return (same_bus && same_addr && same_vid && same_pid) ? 0 : -1; > } > @@ -412,10 +428,17 @@ static void notify_dev_state_change(GUdevClient *self, > 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) { > - /* Found a device that changed its state */ > + GList *found = g_list_find_custom(new_list, dev->data, > compare_libusb_devices); > + if (found == NULL) { > g_udev_device_print(dev->data, add ? "add" : "remove"); > - g_signal_emit(self, signals[UEVENT_SIGNAL], 0, dev->data, add); > + g_udev_notify_device(self, dev->data, add); > + } else if (add) { > + /* keep old existing libusb_device in the new list, when > + usb-dev-manager will maintain its own list of libusb_device, > + these lists will be completely consistent */ > + libusb_device *temp = found->data; > + found->data = dev->data; > + dev->data = temp; Is this chunk required in this commit for proper behaviour the usb code? Or will it be required in some follow-up commit? If it's only going to be required later, I'd squash this bit in the commit where it's needed so that it's clearer why we want this. Apart from these, Acked-by: Christophe Fergeau <cferg...@redhat.com>
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel