<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""> <div><blockquote type="cite" class=""><div class="">On 1 Mar 2016, at 24:06 AM, Jonathon Jongsma <<a href="mailto:jjongsma@redhat.com" class="">jjongsma@redhat.com</a>> wrote:</div> <div class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">On Sun, 2016-02-28 at 11:54 +0200, Dmitry Fleytman wrote:<br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">This patch fixes device list change notification handing logic for cases when more than one device being plugged or unplugged simultaneously. The simplest way to reproduce the problematic scenario is (un)plugging of a usb HUB with a few devices inserted. Signed-off-by: Dmitry Fleytman <<a href="mailto:dmitry@daynix.com" class="">dmitry@daynix.com</a>> --- src/win-usb-dev.c | 82 +++++++++++++++++++----------------------------------- - 1 file changed, 28 insertions(+), 54 deletions(-) diff --git a/src/win-usb-dev.c b/src/win-usb-dev.c index 1e4b2d6..1b4d353 100644 --- a/src/win-usb-dev.c +++ b/src/win-usb-dev.c @@ -34,7 +34,6 @@ struct _GUdevClientPrivate { libusb_context *ctx; - gssize udev_list_size; GList *udev_list; HWND hwnd; }; @@ -196,9 +195,7 @@ g_udev_client_initable_init(GInitable *initable, GCancellable *cancellable, } /* get initial device list */ - priv->udev_list_size = g_udev_client_list_devices(self, &priv->udev_list, - err, __FUNCTION__); - if (priv->udev_list_size < 0) { + if (g_udev_client_list_devices(self, &priv->udev_list, err, __FUNCTION__) < 0) { goto g_udev_client_init_failed; } @@ -339,74 +336,51 @@ static gboolean gudev_devices_are_equal(GUdevDevice *a, GUdevDevice *b) </blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Immediately above this line is the following comment:<br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">/* Assumes each event stands for a single device change (at most) */<br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Since you're changing the function to handle multiple insertions or removals at<br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">once, we should probably remove this comment.<br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""></div></blockquote><div> </div><div>Right, I forgot to drop it. Removed.</div> <blockquote type="cite" class=""><div class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">static void handle_dev_change(GUdevClient *self) { GUdevClientPrivate *priv = self->priv; - GUdevDevice *changed_dev = NULL; - ssize_t dev_count; - int is_dev_change; GError *err = NULL; GList *now_devs = NULL; - 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 */ + GList *prev, *curr; /* iterators for previous and current device lists */ - dev_count = g_udev_client_list_devices(self, &now_devs, &err, - __FUNCTION__); - g_return_if_fail(dev_count >= 0); - - SPICE_DEBUG("number of current devices %"G_GSSIZE_FORMAT - ", I know about %"G_GSSIZE_FORMAT" devices", - dev_count, priv->udev_list_size); - - is_dev_change = dev_count - priv->udev_list_size; - if (is_dev_change == 0) { - g_udev_client_free_device_list(&now_devs); + if(g_udev_client_list_devices(self, &now_devs, &err, __FUNCTION__) < 0) { + g_warning("could not retrieve device list"); return; } - if (is_dev_change > 0) { - llist = now_devs; - slist = priv->udev_list; - } else { - llist = priv->udev_list; - slist = now_devs; - } - - g_udev_device_print_list(llist, "handle_dev_change: long list:"); - g_udev_device_print_list(slist, "handle_dev_change: short list:"); + g_udev_device_print_list(now_devs, "handle_dev_change: current list:"); + g_udev_device_print_list(priv->udev_list, "handle_dev_change: previous list:"); - /* Go over the longer list */ - for (lit = g_list_first(llist); lit != NULL; lit=g_list_next(lit)) { - ldev = lit->data; - /* Look for dev in the shorther list */ - for (sit = g_list_first(slist); sit != NULL; sit=g_list_next(sit)) { - sdev = sit->data; - if (gudev_devices_are_equal(ldev, sdev)) + /* Unregister devices that are not present anymore */ + for (prev = g_list_first(priv->udev_list); prev != NULL; prev = g_list_next(prev)) { + /* Look for dev in the current list */ + for (curr = g_list_first(now_devs); curr != NULL; curr = g_list_next(curr)) { + if (gudev_devices_are_equal(prev->data, curr->data)) </blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">g_list_find_custom() is a possibility here<br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""></div></blockquote><div> </div><div> </div><div>Ok.</div> <blockquote type="cite" class=""><div class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""> break; } - if (sit == NULL) { - /* Found a device which appears only in the longer list */ - changed_dev = ldev; - break; + + if (curr == NULL) { + /* Found a device that was removed */ + g_udev_device_print(prev->data, "<<< USB device removed"); + g_signal_emit(self, signals[UEVENT_SIGNAL], 0, "remove", prev ->data); } } - if (!changed_dev) { - g_warning("couldn't find any device change"); - goto leave; - } + /* Register newly inserted devices */ + for (curr = g_list_first(now_devs); curr != NULL; curr = g_list_next(curr)) { + /* Look for dev in the previous list */ + for (prev = g_list_first(priv->udev_list); prev != NULL; prev = g_list_next(prev)) { + if (gudev_devices_are_equal(prev->data, curr->data)) + break; </blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">and here<br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""></div></blockquote><div> </div><div>Actually these nested loops are similar in both cases so in the next version I moved those to a separate function and used g_list_find_custom() as you suggested.</div> <blockquote type="cite" class=""><div class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">+ } - if (is_dev_change > 0) { - g_udev_device_print(changed_dev, ">>> USB device inserted"); - g_signal_emit(self, signals[UEVENT_SIGNAL], 0, "add", changed_dev); - } else { - g_udev_device_print(changed_dev, "<<< USB device removed"); - g_signal_emit(self, signals[UEVENT_SIGNAL], 0, "remove", changed_dev); + if (prev == NULL) { + /* Found a device that was inserted */ + g_udev_device_print(curr->data, ">>> USB device inserted"); + g_signal_emit(self, signals[UEVENT_SIGNAL], 0, "add", curr ->data); + } } -leave: /* 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; - priv->udev_list_size = dev_count; } static LRESULT CALLBACK wnd_proc(HWND hwnd, UINT message, WPARAM wparam, LPARAM lparam) </blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Acked-by: Jonathon Jongsma <<a href="mailto:jjongsma@redhat.com" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">jjongsma@redhat.com</a><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">></div></blockquote></div> </body></html>