On Mon, Jul 06, 2015 at 08:59:02PM +0300, Kirill Moizik wrote: > wnd_proc callback should not query usb devices in the middle of redirecting > flow. Caller may get inconsistent enumeration results due to device resets > performed by UsbDk and Windows mechanisms handling those resets.
Please wrap all your logs to 72 chars or so. > > Signed-off-by: Kirill Moizik <[email protected]> > --- > src/map-file | 2 ++ > src/win-usb-dev.c | 15 +++++++++++++++ > src/win-usb-dev.h | 2 ++ > 3 files changed, 19 insertions(+) > > diff --git a/src/map-file b/src/map-file > index d5a073f..d0c24a4 100644 > --- a/src/map-file > +++ b/src/map-file > @@ -117,6 +117,8 @@ spice_uri_to_string; > spice_usb_device_get_description; > spice_usb_device_get_libusb_device; > spice_usb_device_get_type; > +spice_g_udev_set_redirecting; > +spice_g_udev_handle_device_change; > spice_usb_device_manager_can_redirect_device; > spice_usb_device_manager_connect_device_async; > spice_usb_device_manager_connect_device_finish; > diff --git a/src/win-usb-dev.c b/src/win-usb-dev.c > index 1e4b2d6..23bea42 100644 > --- a/src/win-usb-dev.c > +++ b/src/win-usb-dev.c > @@ -37,6 +37,7 @@ struct _GUdevClientPrivate { > gssize udev_list_size; > GList *udev_list; > HWND hwnd; > + gboolean redirecting; > }; > > #define G_UDEV_CLIENT_WINCLASS_NAME TEXT("G_UDEV_CLIENT") > @@ -186,6 +187,7 @@ g_udev_client_initable_init(GInitable *initable, > GCancellable *cancellable, > self = G_UDEV_CLIENT(initable); > priv = self->priv; > > + priv->redirecting = FALSE; > rc = libusb_init(&priv->ctx); > if (rc < 0) { > const char *errstr = spice_usbutil_libusb_strerror(rc); > @@ -334,6 +336,11 @@ static gboolean gudev_devices_are_equal(GUdevDevice *a, > GUdevDevice *b) > return (same_pid && same_vid); > } > > +void spice_g_udev_set_redirecting (gboolean val) > +{ > + GUdevClientPrivate *priv = singleton->priv; > + priv->redirecting = val; > +} I still think we should have something like if (priv->redirecting && !val) { handle_dev_change(singleton); } and not export spice_g_udev_handle_device_change() as updating the device list is required anyway when redirecting changes from TRUE to FALSE. Doing it automatically makes using the API less error-prone... > > /* Assumes each event stands for a single device change (at most) */ > static void handle_dev_change(GUdevClient *self) > @@ -347,6 +354,9 @@ static void handle_dev_change(GUdevClient *self) > GList *llist, *slist; /* long-list and short-list*/ > GList *lit, *sit; /* iterators for long-list and short-list */ > GUdevDevice *ldev, *sdev; /* devices on long-list and short-list */ > + if (priv->redirecting == TRUE) { Just if (priv->redirecting) { }, especially as g_udev_set_redirecting() does not do priv->redirecting = !!val; to ensure this only gets set to 0 or 1 Christophe
pgpRuHAgBlK8M.pgp
Description: PGP signature
_______________________________________________ Spice-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/spice-devel
