On Tue, Jul 7, 2015 at 1:47 PM, Christophe Fergeau <[email protected]> wrote:
> 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... > > The problem is not to prevent call handle_dev_change from the usb-device-widget.c I agree with you here, in this case it is better solution. The problem is here static LRESULT CALLBACK wnd_proc(HWND hwnd, UINT message, WPARAM wparam, LPARAM lparam) { /* Only DBT_DEVNODES_CHANGED recieved */ if (message == WM_DEVICECHANGE) { handle_dev_change(singleton); } return DefWindowProc(hwnd, message, wparam, lparam); } It is a registered system callback. The only context available when it is called is GUdevClient singleton object. > > > /* 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 >
_______________________________________________ Spice-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/spice-devel
