[Spice-devel] [spice-gtk v2 10/13] win-usb-dev: maintain list of libusb devices

[Christophe Fergeau]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20190325/ec228e31/attachment.sig>