On Thu, Mar 21, 2019 at 05:16:03PM +0200, Yuri Benditovich wrote:
> > > @@ -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;
> >
> > I'm still annoyed by this slightly complicated code in a method named notify_dev_state_change
> > (more on this below)
> >
> > > }
> > > }
> > > }
> > > @@ -446,7 +469,8 @@ static void handle_dev_change(GUdevClient *self)
> > > /* Unregister devices that are not present anymore */
> > > notify_dev_state_change(self, priv->udev_list, now_devs, FALSE);
> > >
> > > - /* Register newly inserted devices */
> > > + /* report newly inserted devices and swap pointers to existing devices:
> > > + keep old pointers in now_devs list, keep new pointers in udev_list */
> > > 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);
> > > priv->udev_list = now_devs;
> >
> > Maybe these 2 lines + the code to replace new libusb_device pointers
> > with the old ones could be moved to a new
> > g_udev_client_update_device_list(self, now_devs);
> > helper?
> >
>
> From my point of view, this will make the code more complicated,
> as additional procedure should traverse both lists again.
I made an attempt at that, see the 2 patches in answer to this thread.
I did not test them though :-/ Let me know what you think.
Christophe
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel