[Spice-devel] [spice-gtk 2/2] win-usb-dev: Use 'stable' libusb_device pointers in GUdevClient::udev_list

[Yuri Benditovich]

Yes, it looks like this finally does the same thing.
>From my personal point of view, this is less obvious than previous
code, so I would add 2 comments:
that we unreference the device from the new list
that we add reference to the device from the old list, it will be
dereferenced on next line when the old list freed


On Mon, Mar 25, 2019 at 6:17 PM Christophe Fergeau <cfergeau at redhat.com> wrote:
>
> When getting a new list of USB devices after a WM_DEVICECHANGE event,
> libusb_device which were already present before the event may be
> represented by a different instance than the one we got in the new list.
>
> Since other layers of spice-gtk may be using that older instance of
> libusb_device, this commit makes sure that we always keep the older
> instance in GUdevClient::udev_list.
>
> At the moment, this should not be making any difference, but will make
> things more consistent later on.
>
> Based on a patch from Yuri Benditovich.
>
> Signed-off-by: Christophe Fergeau <cfergeau at redhat.com>
> ---
>
> Code is a bit longer this way, but imo it makes more sense to modify the
> list when it's being updated, rather than modifying it during
> notification.
>
>
>  src/win-usb-dev.c | 22 ++++++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/src/win-usb-dev.c b/src/win-usb-dev.c
> index 192ab17f..a16bd3ae 100644
> --- a/src/win-usb-dev.c
> +++ b/src/win-usb-dev.c
> @@ -423,6 +423,25 @@ static gint compare_libusb_devices(gconstpointer a, gconstpointer b)
>      return (same_bus && same_addr && same_vid && same_pid) ? 0 : -1;
>  }
>
> +static void update_device_list(GUdevClient *self, GList *new_device_list)
> +{
> +    GList *dev;
> +
> +    for (dev = g_list_first(new_device_list); dev != NULL; dev = g_list_next(dev)) {
> +        GList *found = g_list_find_custom(self->priv->udev_list, dev->data, compare_libusb_devices);
> +        if (found != NULL) {
> +            /* 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_unref_device(dev->data);
> +            dev->data = libusb_ref_device(found->data);
> +        }
> +    }
> +
> +    g_udev_client_free_device_list(&self->priv->udev_list);
> +    self->priv->udev_list = new_device_list;
> +}
> +
>  static void notify_dev_state_change(GUdevClient *self,
>                                      GList *old_list,
>                                      GList *new_list,
> @@ -469,8 +488,7 @@ static void handle_dev_change(GUdevClient *self)
>      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;
> +    update_device_list(self, now_devs);
>  }
>
>  static LRESULT CALLBACK wnd_proc(HWND hwnd, UINT message, WPARAM wparam, LPARAM lparam)
> --
> 2.21.0
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel