On Mon, Jul 22, 2019 at 12:53 PM Frediano Ziglio <fziglio@xxxxxxxxxx> wrote:
>
> >
> > On Fri, Jul 19, 2019 at 1:14 PM Victor Toso <victortoso@xxxxxxxxxx> wrote:
> > >
> > > Hi,
> > >
> > > On Sun, Jul 14, 2019 at 05:07:38PM +0300, Yuri Benditovich wrote:
> > > > Remove Windows-specific part of hotplug detection from
> > > > usb-device-manager and win-usb-dev and place it into
> > > > USB backend module. Connect the hotplug/unplug detection
> > > > in Windows to the same callbacks already used in Linux.
> > > > This removes significant part of Windows-specific code
> > > > and simpifies things.
> > > > Note regarding 'redirecting' property of usb-device-manager:
> > > > - Previously usb-device-manager under Windows maintained
> > > > 'redirection' property in win-usb-dev
> > > > - Now it maintains its own property in both Windows and
> > > > Linux (for GUI and single redirect at a time)
> > > > - The USB backend maintains its own 'redirecting' field
> > > > in Windows to prevent update of device list when
> > > > libusb opens the device (device redirection causes
> > > > temporary removal of target device)
> > >
> > > Thanks for the description. Only minor things, logic seems fine,
> > > did quick tests only. Would be nice to reduce diff from things
> > > that can be smaller patches whenever needed, function renames or
> > > being moved or variables being dropped for instance so it would
> > > get easier to focus on the actual interesting part of proposal.
> > > This is always mentioned and that's one of the reasons that it
> > > takes time to review.
> > >
> > > > Signed-off-by: Yuri Benditovich <yuri.benditovich@xxxxxxxxxx>
> > > > ---
> > > > meson.build | 2 +-
> > > > src/meson.build | 4 +-
> > > > src/usb-backend.c | 360 ++++++++++++++++++++++----------
> > > > src/usb-backend.h | 8 -
> > > > src/usb-device-manager.c | 61 +-----
> > > > src/win-usb-dev.c | 436 ---------------------------------------
> > > > src/win-usb-dev.h | 84 --------
> > > > 7 files changed, 255 insertions(+), 700 deletions(-)
> > >
> > > Quite happy with the code reduction! That's one thing I was
> > > expecting from introducing usb-backend.[ch]
> > >
> > > > delete mode 100644 src/win-usb-dev.c
> > > > delete mode 100644 src/win-usb-dev.h
> > > >
> > > > diff --git a/meson.build b/meson.build
> > > > index 54160f9..1f6ea6d 100644
> > > > --- a/meson.build
> > > > +++ b/meson.build
> > > > @@ -117,7 +117,7 @@ endforeach
> > > >
> > > > deps = []
> > > > if host_machine.system() == 'windows'
> > > > - deps += ['libws2_32', 'libgdi32']
> > > > + deps += ['libws2_32', 'libgdi32', 'libcomctl32']
> > > > endif
> > > >
> > > > foreach dep : deps
> > > > diff --git a/src/meson.build b/src/meson.build
> > > > index a51d0a9..adcfaec 100644
> > > > --- a/src/meson.build
> > > > +++ b/src/meson.build
> > > > @@ -158,9 +158,7 @@ endif
> > > >
> > > > if spice_gtk_has_usbredir and host_machine.system() == 'windows'
> > > > spice_client_glib_sources += ['usbdk_api.c',
> > > > - 'usbdk_api.h',
> > > > - 'win-usb-dev.c',
> > > > - 'win-usb-dev.h']
> > > > + 'usbdk_api.h']
> > > > endif
> > > >
> > > > #
> > > > diff --git a/src/usb-backend.c b/src/usb-backend.c
> > > > index a2c502d..9964c4f 100644
> > > > --- a/src/usb-backend.c
> > > > +++ b/src/usb-backend.c
> > > > @@ -32,6 +32,9 @@
> > > > #include <libusb.h>
> > > > #include <string.h>
> > > > #include <fcntl.h>
> > > > +#ifdef G_OS_WIN32
> > > > +#include <commctrl.h>
> > > > +#endif
> > > > #include "usbredirhost.h"
> > > > #include "usbredirparser.h"
> > > > #include "spice-util.h"
> > > > @@ -55,6 +58,11 @@ struct _SpiceUsbBackend
> > > > usb_hot_plug_callback hotplug_callback;
> > > > void *hotplug_user_data;
> > > > libusb_hotplug_callback_handle hotplug_handle;
> > > > +#ifdef G_OS_WIN32
> > > > + HANDLE hWnd;
> > > > + libusb_device **libusb_device_list;
> > > > + gint redirecting;
> > > > +#endif
> > > > };
> > > >
> > > > struct _SpiceUsbBackendChannel
> > > > @@ -66,9 +74,229 @@ struct _SpiceUsbBackendChannel
> > > > int rules_count;
> > > > SpiceUsbBackendDevice *attached;
> > > > SpiceUsbredirChannel *user_data;
> > > > + SpiceUsbBackend *backend;
> > > > GError **error;
> > > > };
> > > >
> > >
> > > Some code below is just being moved, makes the diff a little
> > > bigger and review a bit slower. If I comment on code that has
> > > been only moved and you consider the comments relevant, please
> > > address them in different patches.
> > >
> > > > +static gboolean fill_usb_info(SpiceUsbBackendDevice *bdev)
> > >
> > > In several places we have different variable names for giving
> > > structures, it would be good to have it somewhat similar so that
> > > reading the code you don't have to think if dev is libusb_device
> > > or SpiceUsbBackendDevice, etc.
> > >
> > > > +{
> > > > + UsbDeviceInformation *info = &bdev->device_info;
> > > > +
> > > > + struct libusb_device_descriptor desc;
> > > > + libusb_device *libdev = bdev->libusb_device;
> > > > + libusb_get_device_descriptor(libdev, &desc);
> > > > + info->bus = libusb_get_bus_number(libdev);
> > > > + info->address = libusb_get_device_address(libdev);
> > > > + if (info->address == 0xff || /* root hub (HCD) */
> > > > + info->address <= 1 || /* root hub or bad address */
> > > > + (desc.bDeviceClass == LIBUSB_CLASS_HUB) /*hub*/) {
> > > > + return FALSE;
> > > > + }
> > > > +
> > > > + info->vid = desc.idVendor;
> > > > + info->pid = desc.idProduct;
> > > > + info->class = desc.bDeviceClass;
> > > > + info->subclass = desc.bDeviceSubClass;
> > > > + info->protocol = desc.bDeviceProtocol;
> > >
> > > Filling UsbDeviceInformation from libusb_device can be made into
> > > a utility function as this happens here and is_same_libusb_dev(),
> > > but also because it can remove a extra parameter on the later.
> > > Taking a shot at the name, get_device_info_from_libusb_device()!
> > >
> > > > +
> > > > + return TRUE;
> > > > +}
> > > > +
> > > > +static SpiceUsbBackendDevice *allocate_backend_device(libusb_device
> > > > *libdev)
> > > > +{
> > > > + SpiceUsbBackendDevice *dev = g_new0(SpiceUsbBackendDevice, 1);
> > > > + dev->ref_count = 1;
> > > > + dev->libusb_device = libdev;
> > > > + if (!fill_usb_info(dev)) {
> > > > + g_clear_pointer(&dev, g_free);
> > > > + }
> > > > + return dev;
> > > > +}
> > > > +
> > > > +static int LIBUSB_CALL hotplug_callback(libusb_context *ctx,
> > > > + libusb_device *device,
> > > > + libusb_hotplug_event event,
> > >
> > > Perhaps adding lib prefix to libusb related variables, as you did
> > > in the other functions above, would be enough.
> > >
> > > > + void *user_data)
> > > > +{
> > > > + SpiceUsbBackend *be = (SpiceUsbBackend *)user_data;
> > >
> > > No need to cast void*
> > >
> > > > + if (be->hotplug_callback) {
> > >
> > > Should warn if fail, no? This callback should only be called
> > > after spice_usb_backend_register_hotplug() - If you agree,
> > > g_return_val_if_fail() might fit
> > >
> > > > + SpiceUsbBackendDevice *dev;
> > > > + gboolean val = event == LIBUSB_HOTPLUG_EVENT_DEVICE_ARRIVED;
> > >
> > > val -> has_arrived
> > >
> > > > + dev = allocate_backend_device(device);
> > > > + if (dev) {
> > > > + SPICE_DEBUG("created dev %p, usblib dev %p", dev, device);
> > > > + libusb_ref_device(device);
> > > > + be->hotplug_callback(be->hotplug_user_data, dev, val);
> > > > + spice_usb_backend_device_unref(dev);
> > > > + }
> > > > + }
> > > > + return 0;
> > > > +}
> > > > +
> > > > +#ifdef G_OS_WIN32
> > > > +/* Windows-specific: get notification on device change */
> > > > +
> > > > +static gboolean is_same_libusb_dev(libusb_device *dev1,
> > > > + UsbDeviceInformation *info1,
> > > > + libusb_device *dev2)
> > >
> > > This function should just be comparing both libusb_devices, info1
> > > argument is for debug on the caller so can be removed...
> > >
> > > > +{
> > > > + struct libusb_device_descriptor desc;
> > > > + info1->bus = libusb_get_bus_number(dev1);
> > > > + info1->address = libusb_get_device_address(dev1);
> > > > + libusb_get_device_descriptor(dev1, &desc);
> > > > + info1->vid = desc.idVendor;
> > > > + info1->pid = desc.idProduct;
> > > > + if (dev2 == NULL) {
> > > > + return FALSE;
> > > > + }
> > >
> > > ... I think the first check is if either libdev1 or libdev2 are NULL
> > > and return FALSE, otherwise you can then compare after
> > >
>
> It looks like this test is just because the caller mistake,
> maybe the caller should never call this function passing NULLs ?
>
> > > info1 = get_device_info_from_libusb_device(dev1);
> > > info2 = get_device_info_from_libusb_device(dev2);
> > >
> > > > + libusb_get_device_descriptor(dev2, &desc);
> > > > + return info1->bus == libusb_get_bus_number(dev2) &&
> > > > + info1->address == libusb_get_device_address(dev2) &&
> > > > + info1->vid == desc.idVendor &&
> > > > + info1->pid == desc.idProduct;
> > > > +}
> > > > +
> > > > +static int compare_dev_list(SpiceUsbBackend *be,
> > > > + libusb_device **l1,
> > > > + libusb_device **l2,
> > >
> > > Hmm const them? Or..
> > >
>
> l1 and l2 seems a bit short, maybe list1 and list2 ?
> I think best type would be "const libusb_device* const *".
>
const libusb_device* will not work here as in libusb.h even getting
the descriptor requires libusb_device *
libusb device * const * will work
> > > > + gboolean value)
> > >
> > > value -> has_arrived
> > >
> > > .. This function tries to find elements of list1 in list2 and calls
> > > hotplug_callback() for every element not found. This is a bit
> > > more than the name of the function suggests.
> > >
> > >
> > > I would rather have it as
> > > GList* dev_list_difference(libusb_device **libdevp1,
> > > libusb_device **libdevp2);
> > >
> > > and the caller would call hotplug_callback() and
> > > g_list_free() it.
> > >
> >
> > I never allocate memory when there is easy way to deal without it
> >
>
> I agree a bit with both. The split is just a not so useful complication.
> The name is confusing however. I would expect such a function to not
> change anything and return is the lists are the same, instead the
> difference is computed and for each device hotplug_callback is called.
> The number of added/removed devices are returned.
>
> Maybe a name could be "hotplug_added_devices" ?
>
> > > > +{
> > > > + int res = 0;
> > > > + while (*l1) {
>
> Maybe
>
> for (; *l1; ++l1) {
>
> and remove l1++ below?
>
> > > > + gboolean found = 0;
>
> use FALSE? I would persoanally use bool.
>
> > > > + uint32_t n = 0;
> > > > + UsbDeviceInformation info1;
> > > > + while (!found) {
>
> Maybe
>
> for (libusb_device **l = l2; *l; ++l) {
>
> and remove n above and n++ below ??
>
> > > > + found = is_same_libusb_dev(*l1, &info1, l2[n]);
> > > > + if (l2[n] == NULL) {
> > > > + break;
> > > > + }
>
> Maybe this check should be done before the is_same_libusb_dev call
> so is_same_libusb_dev won't have to deal with NULL pointers?
>
> I understand that potentially is_same_libusb_dev is not called
> so info1 is not initialized but looks like is_same_libusb_dev is
> doing 2 things instead of one as Victor mentioned. And more bad
> is doing 2 things just for debug sake.
> IMHO all that additional computation in some part of code to make
> other code happy make stuff confusing, it's fine now but the code
> is less maintainable, when in the future we would maintain or when
> new people came the code won't make much sense.
>
> > > > + n++;
> > > > + }
> > > > + if (!found) {
> > > > + SPICE_DEBUG("%s %04X:%04X at %d:%d", value ? "adding" :
> > > > "removing",
> > > > + info1.vid, info1.pid, info1.bus, info1.address);
>
> info1 is used only here for debugging but compute for all l2 items.
>
> > > > + hotplug_callback(NULL, *l1,
> > > > + value ? LIBUSB_HOTPLUG_EVENT_DEVICE_ARRIVED :
> > > > LIBUSB_HOTPLUG_EVENT_DEVICE_LEFT,
> > > > + be);
> > > > + res++;
> > > > + }
> > > > + l1++;
> > > > + }
> > > > + return res;
> > > > +}
> > > > +
> > > > +static LRESULT subclass_proc(HWND hWnd, UINT uMsg, WPARAM wParam, LPARAM
> > > > lParam,
> > > > + UINT_PTR uIdSubclass, DWORD_PTR dwRefData)
> > > > +{
> > > > + SpiceUsbBackend *be = (SpiceUsbBackend *)dwRefData;
> > > > + if (uMsg == WM_DEVICECHANGE && !be->redirecting) {
> > > > + libusb_device **new_list = NULL;
> > > > + libusb_get_device_list(be->libusb_context, &new_list);
> > > > + if (new_list) {
> > > > + int res = compare_dev_list(be, be->libusb_device_list,
> > > > new_list, FALSE);
> > > > + res += compare_dev_list(be, new_list,
> > > > be->libusb_device_list, TRUE);
> > > > + if (res) {
> > > > + /* there were changes in device tree */
> > > > + libusb_free_device_list(be->libusb_device_list, TRUE);
> > > > + be->libusb_device_list = new_list;
> > > > + } else {
> > > > + libusb_free_device_list(new_list, TRUE);
> > > > + }
> > > > + } else {
> > > > + g_warn_if_reached();
> > > > + }
> > > > + }
> > > > + return DefSubclassProc(hWnd, uMsg, wParam, lParam);
> > > > +}
> > > > +
> > > > +static void disable_hotplug_support(SpiceUsbBackend *be)
> > > > +{
> > > > + if (be->hWnd) {
> > > > + DestroyWindow(be->hWnd);
> > > > + be->hWnd = NULL;
> > > > + }
> > > > + if (be->libusb_device_list) {
> > > > + libusb_free_device_list(be->libusb_device_list, TRUE);
> > > > + be->libusb_device_list = NULL;
> > > > + }
> > > > +}
> > > > +
> > > > +static int enable_hotplug_support(SpiceUsbBackend *be, const char
> > > > **error_on_enable)
> > > > +{
> > > > + long win_err;
> > > > + libusb_device **dev_list = NULL;
> > > > + libusb_device *dummy = NULL;
> > > > +
> > > > + libusb_get_device_list(be->libusb_context, &dev_list);
> > > > + if (!dev_list) {
> > > > + *error_on_enable = "Getting device list";
> > > > + goto error;
> > > > + }
> > > > + be->hWnd = CreateWindow("Static", NULL, 0, 0, 0, 0, 0, NULL, NULL,
> > > > NULL, NULL);
> > >
> > > Not sure if can be important but it was G_UDEV_CLIENT_WINCLASS_NAME before,
> > >
> > > #define G_UDEV_CLIENT_WINCLASS_NAME TEXT("G_UDEV_CLIENT")
> > >
> > > and I notice that you dropped RegisterClass call, not needed
> > > anymore?
> > >
> > >
> >
> > 'Static' is standard class, starting from XP there is easy way to work
> > without registering own class.
> >
>
> I think static was present in Windows 3.1 too :-)
Yes, but not SubclassWindow, prior to XP for subclassing you needed to
use SetWindowLong
> Make sense to me, but a small comment won't hurt, I too stopped here and
> thought "why static" ?
>
> > > > + if (!be->hWnd) {
> > > > + *error_on_enable = "CreateWindow";
> > > > + goto error;
> > > > + }
> > > > + if (!SetWindowSubclass(be->hWnd, subclass_proc, 0, (DWORD_PTR)be)) {
> > > > + *error_on_enable = "SetWindowSubclass";
> > > > + goto error;
> > > > + }
> > > > + be->hotplug_handle = 1;
> > > > + be->libusb_device_list = dev_list;
> > > > +
> > > > + compare_dev_list(be, be->libusb_device_list, &dummy, TRUE);
> > >
> > > What's the point of this function call? to call
> > > hotplug_callback() on dev_list? You might as well just do it here
> > > with a comment
> > >
> > The point is that this takes one line of code, just call
> > hotplug_callback takes more.
> > Procedures exist for reuse, I think
> >
>
> A
>
> hotplug_added_devices(be, be->libusb_device_list, &empty_device_list, TRUE);
>
> would be more readable.
>
> > > > +
> > > > + return LIBUSB_SUCCESS;
> > > > +error:
> > > > + win_err = GetLastError();
> > > > + if (!win_err) {
> > > > + win_err = -1;
> > > > + }
> > > > + g_warning("%s failed: %ld", *error_on_enable, win_err);
> > > > + if (dev_list) {
> > > > + libusb_free_device_list(dev_list, TRUE);
> > > > + }
> > > > + return win_err;
> > > > +}
> > > > +
> > > > +static void set_redirecting(SpiceUsbBackend *be, gboolean on)
> > > > +{
> > > > + gboolean no_redir;
> > > > + if (on) {
> > > > + g_atomic_int_inc(&be->redirecting);
> > > > + } else {
> > > > + no_redir = g_atomic_int_dec_and_test(&be->redirecting);
> > >
> > > You can move declaration of no_redir here
> > >
> > > > + if (no_redir && be->hWnd) {
> > > > + PostMessage(be->hWnd, WM_DEVICECHANGE, 0, 0);
> > > > + }
> > > > + }
> > > > +}
> > > > +
> > > > +#else
> > > > +/* Linux-specific: use hot callback from libusb */
> > > > +
> > > > +static void disable_hotplug_support(SpiceUsbBackend *be)
> > > > +{
> > > > + libusb_hotplug_deregister_callback(be->libusb_context,
> > > > be->hotplug_handle);
> > > > +}
> > > > +
> > > > +static int enable_hotplug_support(SpiceUsbBackend *be, const char
> > > > **error_on_enable)
> > > > +{
> > > > + int rc = 0;
> > > > + rc = libusb_hotplug_register_callback(be->libusb_context,
> > > > + LIBUSB_HOTPLUG_EVENT_DEVICE_ARRIVED |
> > > > LIBUSB_HOTPLUG_EVENT_DEVICE_LEFT,
> > > > + LIBUSB_HOTPLUG_ENUMERATE, LIBUSB_HOTPLUG_MATCH_ANY,
> > > > + LIBUSB_HOTPLUG_MATCH_ANY, LIBUSB_HOTPLUG_MATCH_ANY,
> > > > + hotplug_callback, be, &be->hotplug_handle);
> > > > + *error_on_enable = libusb_strerror(rc);
> > > > + return rc;
> > > > +}
> > > > +
> > > > +#define set_redirecting(...)
> > >
> > > ok
> > >
>
> Usually I prefer static inline empty function for type safety but
> fine with the macro. The empty expansion could be a problem, some
> compilers for instance give a warning if they find 2 semicolon in
> a row.
>
> > > > +
> > > > +#endif
> > > > +
> > > > /* lock functions for usbredirhost and usbredirparser */
> > > > static void *usbredir_alloc_lock(void) {
> > > > GMutex *mutex;
> > > > @@ -131,41 +359,6 @@ gboolean
> > > > spice_usb_backend_device_isoch(SpiceUsbBackendDevice *dev)
> > > > return isoc_found;
> > > > }
> > > >
> > > > -static gboolean fill_usb_info(SpiceUsbBackendDevice *bdev)
> > > > -{
> > > > - UsbDeviceInformation *info = &bdev->device_info;
> > > > -
> > > > - struct libusb_device_descriptor desc;
> > > > - libusb_device *libdev = bdev->libusb_device;
> > > > - libusb_get_device_descriptor(libdev, &desc);
> > > > - info->bus = libusb_get_bus_number(libdev);
> > > > - info->address = libusb_get_device_address(libdev);
> > > > - if (info->address == 0xff || /* root hub (HCD) */
> > > > - info->address <= 1 || /* root hub or bad address */
> > > > - (desc.bDeviceClass == LIBUSB_CLASS_HUB) /*hub*/) {
> > > > - return FALSE;
> > > > - }
> > > > -
> > > > - info->vid = desc.idVendor;
> > > > - info->pid = desc.idProduct;
> > > > - info->class = desc.bDeviceClass;
> > > > - info->subclass = desc.bDeviceSubClass;
> > > > - info->protocol = desc.bDeviceProtocol;
> > > > -
> > > > - return TRUE;
> > > > -}
> > > > -
> > > > -static SpiceUsbBackendDevice *allocate_backend_device(libusb_device
> > > > *libdev)
> > > > -{
> > > > - SpiceUsbBackendDevice *dev = g_new0(SpiceUsbBackendDevice, 1);
> > > > - dev->ref_count = 1;
> > > > - dev->libusb_device = libdev;
> > > > - if (!fill_usb_info(dev)) {
> > > > - g_clear_pointer(&dev, g_free);
> > > > - }
> > > > - return dev;
> > > > -}
> > > > -
> > > > static gboolean is_channel_ready(SpiceUsbredirChannel *channel)
> > > > {
> > > > return spice_channel_get_state(SPICE_CHANNEL(channel)) ==
> > > > SPICE_CHANNEL_STATE_READY;
> > > > @@ -237,31 +430,11 @@ void
> > > > spice_usb_backend_interrupt_event_handler(SpiceUsbBackend *be)
> > > > }
> > > > }
> > > >
> > > > -static int LIBUSB_CALL hotplug_callback(libusb_context *ctx,
> > > > - libusb_device *device,
> > > > - libusb_hotplug_event event,
> > > > - void *user_data)
> > > > -{
> > > > - SpiceUsbBackend *be = (SpiceUsbBackend *)user_data;
> > > > - if (be->hotplug_callback) {
> > > > - SpiceUsbBackendDevice *dev;
> > > > - gboolean val = event == LIBUSB_HOTPLUG_EVENT_DEVICE_ARRIVED;
> > > > - dev = allocate_backend_device(device);
> > > > - if (dev) {
> > > > - SPICE_DEBUG("created dev %p, usblib dev %p", dev, device);
> > > > - libusb_ref_device(device);
> > > > - be->hotplug_callback(be->hotplug_user_data, dev, val);
> > > > - spice_usb_backend_device_unref(dev);
> > > > - }
> > > > - }
> > > > - return 0;
> > > > -}
> > > > -
> > > > void spice_usb_backend_deregister_hotplug(SpiceUsbBackend *be)
> > > > {
> > > > g_return_if_fail(be != NULL);
> > > > if (be->hotplug_handle) {
> > > > - libusb_hotplug_deregister_callback(be->libusb_context,
> > > > be->hotplug_handle);
> > > > + disable_hotplug_support(be);
> > > > be->hotplug_handle = 0;
> > > > }
> > > > be->hotplug_callback = NULL;
> > > > @@ -272,17 +445,15 @@ gboolean
> > > > spice_usb_backend_register_hotplug(SpiceUsbBackend *be,
> > > > usb_hot_plug_callback proc)
> > > > {
> > > > int rc;
> > > > + const char *desc;
> > > > g_return_val_if_fail(be != NULL, FALSE);
> > > >
> > > > be->hotplug_callback = proc;
> > > > be->hotplug_user_data = user_data;
> > > > - rc = libusb_hotplug_register_callback(be->libusb_context,
> > > > - LIBUSB_HOTPLUG_EVENT_DEVICE_ARRIVED |
> > > > LIBUSB_HOTPLUG_EVENT_DEVICE_LEFT,
> > > > - LIBUSB_HOTPLUG_ENUMERATE, LIBUSB_HOTPLUG_MATCH_ANY,
> > > > - LIBUSB_HOTPLUG_MATCH_ANY, LIBUSB_HOTPLUG_MATCH_ANY,
> > > > - hotplug_callback, be, &be->hotplug_handle);
> > > > +
> > > > + rc = enable_hotplug_support(be, &desc);
> > > > +
> > > > if (rc != LIBUSB_SUCCESS) {
> > > > - const char *desc = libusb_strerror(rc);
> > > > g_warning("Error initializing USB hotplug support: %s [%i]",
> > > > desc, rc);
> > > > be->hotplug_callback = NULL;
> > > > return FALSE;
> > > > @@ -301,45 +472,6 @@ void spice_usb_backend_delete(SpiceUsbBackend *be)
> > > > SPICE_DEBUG("%s <<", __FUNCTION__);
> > > > }
> > > >
> > > > -SpiceUsbBackendDevice
> > > > **spice_usb_backend_get_device_list(SpiceUsbBackend *be)
> > > > -{
> > > > - LOUD_DEBUG("%s >>", __FUNCTION__);
> > > > - libusb_device **devlist = NULL, **dev;
> > > > - SpiceUsbBackendDevice *d, **list;
> > > > -
> > > > - int n = 0, index;
> > > > -
> > > > - if (be && be->libusb_context) {
> > > > - libusb_get_device_list(be->libusb_context, &devlist);
> > > > - }
> > > > -
> > > > - /* add all the libusb device that not present in our list */
> > > > - for (dev = devlist; dev && *dev; dev++) {
> > > > - n++;
> > > > - }
> > > > -
> > > > - list = g_new0(SpiceUsbBackendDevice*, n + 1);
> > > > -
> > > > - index = 0;
> > > > -
> > > > - for (dev = devlist; dev && *dev; dev++) {
> > > > - d = allocate_backend_device(*dev);
> > > > - if (!d) {
> > > > - libusb_unref_device(*dev);
> > > > - } else {
> > > > - SPICE_DEBUG("created dev %p, usblib dev %p", d, *dev);
> > > > - list[index++] = d;
> > > > - }
> > > > - }
> > > > -
> > > > - if (devlist) {
> > > > - libusb_free_device_list(devlist, 0);
> > > > - }
> > > > -
> > > > - LOUD_DEBUG("%s <<", __FUNCTION__);
> > > > - return list;
> > > > -}
> > > > -
> > > > const UsbDeviceInformation*
> > > > spice_usb_backend_device_get_info(SpiceUsbBackendDevice *dev)
> > > > {
> > > > return &dev->device_info;
> > > > @@ -350,18 +482,6 @@ gconstpointer
> > > > spice_usb_backend_device_get_libdev(SpiceUsbBackendDevice *dev)
> > > > return dev->libusb_device;
> > > > }
> > > >
> > > > -void spice_usb_backend_free_device_list(SpiceUsbBackendDevice **devlist)
> > > > -{
> > > > - LOUD_DEBUG("%s >>", __FUNCTION__);
> > > > - SpiceUsbBackendDevice **dev;
> > > > - for (dev = devlist; *dev; dev++) {
> > > > - SpiceUsbBackendDevice *d = *dev;
> > > > - spice_usb_backend_device_unref(d);
> > > > - }
> > > > - g_free(devlist);
> > > > - LOUD_DEBUG("%s <<", __FUNCTION__);
> > > > -}
> > > > -
> > > > SpiceUsbBackendDevice
> > > > *spice_usb_backend_device_ref(SpiceUsbBackendDevice *dev)
> > > > {
> > > > LOUD_DEBUG("%s >> %p", __FUNCTION__, dev);
> > > > @@ -529,12 +649,23 @@ gboolean
> > > > spice_usb_backend_channel_attach(SpiceUsbBackendChannel *ch,
> > > > SpiceUsbBackendDevice *dev,
> > > > GError **error)
> > > > {
> > > > + int rc;
> > > > SPICE_DEBUG("%s >> ch %p, dev %p (was attached %p)", __FUNCTION__,
> > > > ch, dev, ch->attached);
> > > >
> > > > g_return_val_if_fail(dev != NULL, FALSE);
> > > >
> > > > libusb_device_handle *handle = NULL;
> > > > - int rc = libusb_open(dev->libusb_device, &handle);
> > > > +
> > > > + /*
> > > > + Under Windows we need to avoid updating
> > > > + list of devices when we are acquiring the device
> > > > + */
> > >
> > > I'd rather have
> > >
> > > /* start comment
> > > * end comment */
> > >
> > > but I don't mind much.
> > >
> > > Apart from those comments, not much to add.
> > >
> > > Victor
> > >
> > > > + set_redirecting(ch->backend, TRUE);
> > > > +
> > > > + rc = libusb_open(dev->libusb_device, &handle);
> > > > +
> > > > + set_redirecting(ch->backend, FALSE);
> > > > +
> > > > if (rc) {
> > > > const char *desc = libusb_strerror(rc);
> > > > g_set_error(error, SPICE_CLIENT_ERROR,
> > > > SPICE_CLIENT_ERROR_FAILED,
> > > > @@ -582,6 +713,7 @@ SpiceUsbBackendChannel
> > > > *spice_usb_backend_channel_new(SpiceUsbBackend *be,
> > > > SPICE_DEBUG("%s >>", __FUNCTION__);
> > > > ch->user_data = SPICE_USBREDIR_CHANNEL(user_data);
> > > > if (be->libusb_context) {
> > > > + ch->backend = be;
> > > > ch->usbredirhost = usbredirhost_open_full(
> > > > be->libusb_context,
> > > > NULL,
> > > > diff --git a/src/usb-backend.h b/src/usb-backend.h
> > > > index cbb73c2..6da3981 100644
> > > > --- a/src/usb-backend.h
> > > > +++ b/src/usb-backend.h
> > > > @@ -56,14 +56,6 @@ enum {
> > > > SpiceUsbBackend *spice_usb_backend_new(GError **error);
> > > > void spice_usb_backend_delete(SpiceUsbBackend *context);
> > > >
> > > > -/*
> > > > -returns newly-allocated null-terminated list of
> > > > -SpiceUsbBackendDevice pointers.
> > > > -The caller must call spice_usb_backend_free_device_list
> > > > -after it finishes list processing
> > > > -*/
> > > > -SpiceUsbBackendDevice
> > > > **spice_usb_backend_get_device_list(SpiceUsbBackend *backend);
> > > > -void spice_usb_backend_free_device_list(SpiceUsbBackendDevice
> > > > **devlist);
> > > > gboolean spice_usb_backend_handle_events(SpiceUsbBackend *be);
> > > > void spice_usb_backend_interrupt_event_handler(SpiceUsbBackend *be);
> > > > gboolean spice_usb_backend_register_hotplug(SpiceUsbBackend *be,
> > > > diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c
> > > > index 9aba275..9300ad2 100644
> > > > --- a/src/usb-device-manager.c
> > > > +++ b/src/usb-device-manager.c
> > > > @@ -29,7 +29,6 @@
> > > > #ifdef G_OS_WIN32
> > > > #include <windows.h>
> > > > #include "usbdk_api.h"
> > > > -#include "win-usb-dev.h"
> > > > #endif
> > > >
> > > > #include "channel-usbredir-priv.h"
> > > > @@ -101,12 +100,10 @@ struct _SpiceUsbDeviceManagerPrivate {
> > > > struct usbredirfilter_rule *redirect_on_connect_rules;
> > > > int auto_conn_filter_rules_count;
> > > > int redirect_on_connect_rules_count;
> > > > + gboolean redirecting;
> > > > #ifdef G_OS_WIN32
> > > > - GUdevClient *udev;
> > > > usbdk_api_wrapper *usbdk_api;
> > > > HANDLE usbdk_hider_handle;
> > > > -#else
> > > > - gboolean redirecting; /* Handled by GUdevClient in the gudev case */
> > > > #endif
> > > > GPtrArray *devices;
> > > > GPtrArray *channels;
> > > > @@ -139,16 +136,9 @@ static void channel_destroy(SpiceSession *session,
> > > > SpiceChannel *channel,
> > > > gpointer user_data);
> > > > static void channel_event(SpiceChannel *channel, SpiceChannelEvent
> > > > event,
> > > > gpointer user_data);
> > > > -#ifdef G_OS_WIN32
> > > > -static void spice_usb_device_manager_uevent_cb(GUdevClient *client,
> > > > - SpiceUsbBackendDevice
> > > > *udevice,
> > > > - gboolean add,
> > > > - gpointer
> > > > user_data);
> > > > -#else
> > > > static void spice_usb_device_manager_hotplug_cb(void *user_data,
> > > > SpiceUsbBackendDevice
> > > > *dev,
> > > > gboolean added);
> > > > -#endif
> > > > static void spice_usb_device_manager_check_redir_on_connect(
> > > > SpiceUsbDeviceManager *self, SpiceChannel *channel);
> > > >
> > > > @@ -190,11 +180,7 @@ G_DEFINE_BOXED_TYPE(SpiceUsbDevice,
> > > > spice_usb_device,
> > > > static void
> > > > _set_redirecting(SpiceUsbDeviceManager *self, gboolean is_redirecting)
> > > > {
> > > > -#ifdef G_OS_WIN32
> > > > - g_object_set(self->priv->udev, "redirecting", is_redirecting, NULL);
> > > > -#else
> > > > self->priv->redirecting = is_redirecting;
> > > > -#endif
> > > > }
> > > >
> > > > #else
> > > > @@ -215,10 +201,6 @@ gboolean
> > > > spice_usb_device_manager_is_redirecting(SpiceUsbDeviceManager *self)
> > > > {
> > > > #ifndef USE_USBREDIR
> > > > return FALSE;
> > > > -#elif defined(G_OS_WIN32)
> > > > - gboolean redirecting;
> > > > - g_object_get(self->priv->udev, "redirecting", &redirecting, NULL);
> > > > - return redirecting;
> > > > #else
> > > > return self->priv->redirecting;
> > > > #endif
> > > > @@ -267,29 +249,18 @@ static gboolean
> > > > spice_usb_device_manager_initable_init(GInitable *initable,
> > > > GList *list;
> > > > GList *it;
> > > >
> > > > - /* Start listening for usb devices plug / unplug */
> > > > -#ifdef G_OS_WIN32
> > > > - priv->udev = g_udev_client_new();
> > > > - if (priv->udev == NULL) {
> > > > - g_warning("Error initializing GUdevClient");
> > > > - return FALSE;
> > > > - }
> > > > - priv->context = g_udev_client_get_context(priv->udev);
> > > > - g_signal_connect(G_OBJECT(priv->udev), "uevent",
> > > > - G_CALLBACK(spice_usb_device_manager_uevent_cb),
> > > > self);
> > > > - /* Do coldplug (detection of already connected devices) */
> > > > - g_udev_client_report_devices(priv->udev);
> > > > -#else
> > > > /* Initialize libusb */
> > > > priv->context = spice_usb_backend_new(err);
> > > > if (!priv->context) {
> > > > return FALSE;
> > > > }
> > > >
> > > > + /* Start listening for usb devices plug / unplug */
> > > > if (!spice_usb_backend_register_hotplug(priv->context, self,
> > > > spice_usb_device_manager_hotplug_cb))
> > > > {
> > > > return FALSE;
> > > > }
> > > > +#ifndef G_OS_WIN32
> > > > spice_usb_device_manager_start_event_listening(self, NULL);
> > > > #endif
> > > >
> > > > @@ -324,8 +295,9 @@ static void spice_usb_device_manager_dispose(GObject
> > > > *gobject)
> > > > g_warn_if_reached();
> > > > g_atomic_int_set(&priv->event_thread_run, FALSE);
> > > > }
> > > > - spice_usb_backend_deregister_hotplug(priv->context);
> > > > #endif
> > > > + spice_usb_backend_deregister_hotplug(priv->context);
> > > > +
> > > > if (priv->event_thread) {
> > > > g_warn_if_fail(g_atomic_int_get(&priv->event_thread_run) ==
> > > > FALSE);
> > > > g_atomic_int_set(&priv->event_thread_run, FALSE);
> > > > @@ -350,14 +322,10 @@ static void
> > > > spice_usb_device_manager_finalize(GObject *gobject)
> > > > if (priv->devices) {
> > > > g_ptr_array_unref(priv->devices);
> > > > }
> > > > -#ifdef G_OS_WIN32
> > > > - g_clear_object(&priv->udev);
> > > > -#endif
> > > > g_return_if_fail(priv->event_thread == NULL);
> > > > -#ifndef G_OS_WIN32
> > > > - if (priv->context)
> > > > + if (priv->context) {
> > > > spice_usb_backend_delete(priv->context);
> > > > -#endif
> > > > + }
> > > > free(priv->auto_conn_filter_rules);
> > > > free(priv->redirect_on_connect_rules);
> > > > #ifdef G_OS_WIN32
> > > > @@ -891,20 +859,6 @@ static void
> > > > spice_usb_device_manager_remove_dev(SpiceUsbDeviceManager *self,
> > > > spice_usb_device_unref(device);
> > > > }
> > > >
> > > > -#ifdef G_OS_WIN32
> > > > -static void spice_usb_device_manager_uevent_cb(GUdevClient *client,
> > > > - SpiceUsbBackendDevice
> > > > *bdev,
> > > > - gboolean add,
> > > > - gpointer
> > > > user_data)
> > > > -{
> > > > - SpiceUsbDeviceManager *self = SPICE_USB_DEVICE_MANAGER(user_data);
> > > > -
> > > > - if (add)
> > > > - spice_usb_device_manager_add_dev(self, bdev);
> > > > - else
> > > > - spice_usb_device_manager_remove_dev(self, bdev);
> > > > -}
> > > > -#else
> > > > struct hotplug_idle_cb_args {
> > > > SpiceUsbDeviceManager *self;
> > > > SpiceUsbBackendDevice *device;
> > > > @@ -940,7 +894,6 @@ static void spice_usb_device_manager_hotplug_cb(void
> > > > *user_data,
> > > > args->added = added;
> > > > g_idle_add(spice_usb_device_manager_hotplug_idle_cb, args);
> > > > }
> > > > -#endif // USE_USBREDIR
> > > >
> > > > static void spice_usb_device_manager_channel_connect_cb(
> > > > GObject *gobject, GAsyncResult *channel_res, gpointer user_data)
> > > > diff --git a/src/win-usb-dev.c b/src/win-usb-dev.c
> > > > deleted file mode 100644
> > > > index c2a5115..0000000
> > > > --- a/src/win-usb-dev.c
> > > > +++ /dev/null
> > > > @@ -1,436 +0,0 @@
> > > > -/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
> > > > -/*
> > > > - Copyright (C) 2012 Red Hat, Inc.
> > > > -
> > > > - Red Hat Authors:
> > > > - Arnon Gilboa <agilboa@xxxxxxxxxx>
> > > > - Uri Lublin <uril@xxxxxxxxxx>
> > > > -
> > > > - This library is free software; you can redistribute it and/or
> > > > - modify it under the terms of the GNU Lesser General Public
> > > > - License as published by the Free Software Foundation; either
> > > > - version 2.1 of the License, or (at your option) any later version.
> > > > -
> > > > - This library is distributed in the hope that it will be useful,
> > > > - but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > > - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > > > - Lesser General Public License for more details.
> > > > -
> > > > - You should have received a copy of the GNU Lesser General Public
> > > > - License along with this library; if not, see
> > > > <http://www.gnu.org/licenses/>.
> > > > -*/
> > > > -
> > > > -#include "config.h"
> > > > -
> > > > -#include <windows.h>
> > > > -#include "win-usb-dev.h"
> > > > -#include "spice-marshal.h"
> > > > -#include "spice-util.h"
> > > > -
> > > > -enum {
> > > > - PROP_0,
> > > > - PROP_REDIRECTING
> > > > -};
> > > > -
> > > > -struct _GUdevClientPrivate {
> > > > - SpiceUsbBackend *ctx;
> > > > - GList *udev_list;
> > > > - HWND hwnd;
> > > > - gboolean redirecting;
> > > > -};
> > > > -
> > > > -#define G_UDEV_CLIENT_WINCLASS_NAME TEXT("G_UDEV_CLIENT")
> > > > -
> > > > -static void g_udev_client_initable_iface_init(GInitableIface *iface);
> > > > -
> > > > -G_DEFINE_TYPE_WITH_CODE(GUdevClient, g_udev_client, G_TYPE_OBJECT,
> > > > - G_ADD_PRIVATE(GUdevClient)
> > > > - G_IMPLEMENT_INTERFACE(G_TYPE_INITABLE,
> > > > g_udev_client_initable_iface_init))
> > > > -
> > > > -enum
> > > > -{
> > > > - UEVENT_SIGNAL,
> > > > - LAST_SIGNAL,
> > > > -};
> > > > -
> > > > -static guint signals[LAST_SIGNAL] = { 0, };
> > > > -static GUdevClient *singleton = NULL;
> > > > -
> > > > -static LRESULT CALLBACK wnd_proc(HWND hwnd, UINT message, WPARAM wparam,
> > > > LPARAM lparam);
> > > > -
> > > > -//uncomment to debug gudev device lists.
> > > > -//#define DEBUG_GUDEV_DEVICE_LISTS
> > > > -
> > > > -#ifdef DEBUG_GUDEV_DEVICE_LISTS
> > > > -static void g_udev_device_print_list(GList *l, const gchar *msg);
> > > > -#else
> > > > -static void g_udev_device_print_list(GList *l, const gchar *msg) {}
> > > > -#endif
> > > > -static void g_udev_device_print(SpiceUsbBackendDevice *dev, const gchar
> > > > *msg);
> > > > -
> > > > -GQuark g_udev_client_error_quark(void)
> > > > -{
> > > > - return g_quark_from_static_string("win-gudev-client-error-quark");
> > > > -}
> > > > -
> > > > -GUdevClient *g_udev_client_new(void)
> > > > -{
> > > > - if (singleton != NULL)
> > > > - return g_object_ref(singleton);
> > > > -
> > > > - singleton = g_initable_new(G_UDEV_TYPE_CLIENT, NULL, NULL, NULL);
> > > > - return singleton;
> > > > -}
> > > > -
> > > > -SpiceUsbBackend *g_udev_client_get_context(GUdevClient *client)
> > > > -{
> > > > - return client->priv->ctx;
> > > > -}
> > > > -
> > > > -/*
> > > > - * devs [in,out] an empty devs list in, full devs list out
> > > > - * Returns: number-of-devices, or a negative value on error.
> > > > - */
> > > > -static ssize_t
> > > > -g_udev_client_list_devices(GUdevClient *self, GList **devs,
> > > > - GError **err, const gchar *name)
> > > > -{
> > > > - SpiceUsbBackendDevice **lusb_list, **dev;
> > > > - GUdevClientPrivate *priv;
> > > > - ssize_t n;
> > > > -
> > > > - g_return_val_if_fail(G_UDEV_IS_CLIENT(self), -1);
> > > > - g_return_val_if_fail(devs != NULL, -2);
> > > > -
> > > > - priv = self->priv;
> > > > -
> > > > - g_return_val_if_fail(self->priv->ctx != NULL, -3);
> > > > -
> > > > - lusb_list = spice_usb_backend_get_device_list(priv->ctx);
> > > > - if (!lusb_list) {
> > > > - g_set_error(err, G_UDEV_CLIENT_ERROR,
> > > > G_UDEV_CLIENT_LIBUSB_FAILED,
> > > > - "%s: Error getting USB device list", name);
> > > > - return -4;
> > > > - }
> > > > -
> > > > - n = 0;
> > > > - for (dev = lusb_list; *dev; dev++) {
> > > > - *devs = g_list_prepend(*devs,
> > > > spice_usb_backend_device_ref(*dev));
> > > > - n++;
> > > > - }
> > > > - spice_usb_backend_free_device_list(lusb_list);
> > > > -
> > > > - return n;
> > > > -}
> > > > -
> > > > -static void unreference_backend_device(gpointer data)
> > > > -{
> > > > - spice_usb_backend_device_unref((SpiceUsbBackendDevice *)data);
> > > > -}
> > > > -
> > > > -static void g_udev_client_free_device_list(GList **devs)
> > > > -{
> > > > - g_return_if_fail(devs != NULL);
> > > > - if (*devs) {
> > > > - g_list_free_full(*devs, unreference_backend_device);
> > > > - *devs = NULL;
> > > > - }
> > > > -}
> > > > -
> > > > -
> > > > -static gboolean
> > > > -g_udev_client_initable_init(GInitable *initable, GCancellable
> > > > *cancellable,
> > > > - GError **err)
> > > > -{
> > > > - GUdevClient *self;
> > > > - GUdevClientPrivate *priv;
> > > > - WNDCLASS wcls;
> > > > -
> > > > - g_return_val_if_fail(G_UDEV_IS_CLIENT(initable), FALSE);
> > > > - g_return_val_if_fail(cancellable == NULL, FALSE);
> > > > -
> > > > - self = G_UDEV_CLIENT(initable);
> > > > - priv = self->priv;
> > > > -
> > > > - priv->ctx = spice_usb_backend_new(err);
> > > > - if (!priv->ctx) {
> > > > - return FALSE;
> > > > - }
> > > > -
> > > > -#ifdef G_OS_WIN32
> > > > -#if LIBUSB_API_VERSION >= 0x01000106
> > > > - libusb_set_option(priv->ctx, LIBUSB_OPTION_USE_USBDK);
> > > > -#endif
> > > > -#endif
> > > > -
> > > > - /* get initial device list */
> > > > - if (g_udev_client_list_devices(self, &priv->udev_list, err,
> > > > __FUNCTION__) < 0) {
> > > > - goto g_udev_client_init_failed;
> > > > - }
> > > > -
> > > > - g_udev_device_print_list(priv->udev_list, "init: first list is: ");
> > > > -
> > > > - /* create a hidden window */
> > > > - memset(&wcls, 0, sizeof(wcls));
> > > > - wcls.lpfnWndProc = wnd_proc;
> > > > - wcls.lpszClassName = G_UDEV_CLIENT_WINCLASS_NAME;
> > > > - if (!RegisterClass(&wcls)) {
> > > > - DWORD e = GetLastError();
> > > > - g_warning("RegisterClass failed , %ld", (long)e);
> > > > - g_set_error(err, G_UDEV_CLIENT_ERROR,
> > > > G_UDEV_CLIENT_WINAPI_FAILED,
> > > > - "RegisterClass failed: %ld", (long)e);
> > > > - goto g_udev_client_init_failed;
> > > > - }
> > > > - priv->hwnd = CreateWindow(G_UDEV_CLIENT_WINCLASS_NAME,
> > > > - NULL, 0, 0, 0, 0, 0, NULL, NULL, NULL,
> > > > NULL);
> > > > - if (!priv->hwnd) {
> > > > - DWORD e = GetLastError();
> > > > - g_warning("CreateWindow failed: %ld", (long)e);
> > > > - g_set_error(err, G_UDEV_CLIENT_ERROR,
> > > > G_UDEV_CLIENT_LIBUSB_FAILED,
> > > > - "CreateWindow failed: %ld", (long)e);
> > > > - goto g_udev_client_init_failed_unreg;
> > > > - }
> > > > -
> > > > - return TRUE;
> > > > -
> > > > - g_udev_client_init_failed_unreg:
> > > > - UnregisterClass(G_UDEV_CLIENT_WINCLASS_NAME, NULL);
> > > > - g_udev_client_init_failed:
> > > > - return FALSE;
> > > > -}
> > > > -
> > > > -static void g_udev_client_initable_iface_init(GInitableIface *iface)
> > > > -{
> > > > - iface->init = g_udev_client_initable_init;
> > > > -}
> > > > -
> > > > -static void report_one_device(gpointer data, gpointer self)
> > > > -{
> > > > - g_signal_emit(self, signals[UEVENT_SIGNAL], 0, data, TRUE);
> > > > -}
> > > > -
> > > > -void g_udev_client_report_devices(GUdevClient *self)
> > > > -{
> > > > - g_list_foreach(self->priv->udev_list, report_one_device, self);
> > > > -}
> > > > -
> > > > -static void g_udev_client_init(GUdevClient *self)
> > > > -{
> > > > - self->priv = g_udev_client_get_instance_private(self);
> > > > -}
> > > > -
> > > > -static void g_udev_client_finalize(GObject *gobject)
> > > > -{
> > > > - GUdevClient *self = G_UDEV_CLIENT(gobject);
> > > > - GUdevClientPrivate *priv = self->priv;
> > > > -
> > > > - singleton = NULL;
> > > > - DestroyWindow(priv->hwnd);
> > > > - UnregisterClass(G_UDEV_CLIENT_WINCLASS_NAME, NULL);
> > > > - g_udev_client_free_device_list(&priv->udev_list);
> > > > -
> > > > - /* free backend context */
> > > > - g_warn_if_fail(priv->ctx != NULL);
> > > > - spice_usb_backend_delete(priv->ctx);
> > > > -
> > > > - /* Chain up to the parent class */
> > > > - if (G_OBJECT_CLASS(g_udev_client_parent_class)->finalize)
> > > > - G_OBJECT_CLASS(g_udev_client_parent_class)->finalize(gobject);
> > > > -}
> > > > -
> > > > -static void g_udev_client_get_property(GObject *gobject,
> > > > - guint prop_id,
> > > > - GValue *value,
> > > > - GParamSpec *pspec)
> > > > -{
> > > > - GUdevClient *self = G_UDEV_CLIENT(gobject);
> > > > - GUdevClientPrivate *priv = self->priv;
> > > > -
> > > > - switch (prop_id) {
> > > > - case PROP_REDIRECTING:
> > > > - g_value_set_boolean(value, priv->redirecting);
> > > > - break;
> > > > - default:
> > > > - G_OBJECT_WARN_INVALID_PROPERTY_ID(gobject, prop_id, pspec);
> > > > - break;
> > > > - }
> > > > -}
> > > > -
> > > > -static void handle_dev_change(GUdevClient *self);
> > > > -
> > > > -static void g_udev_client_set_property(GObject *gobject,
> > > > - guint prop_id,
> > > > - const GValue *value,
> > > > - GParamSpec *pspec)
> > > > -{
> > > > - GUdevClient *self = G_UDEV_CLIENT(gobject);
> > > > - GUdevClientPrivate *priv = self->priv;
> > > > - gboolean old_val;
> > > > -
> > > > - switch (prop_id) {
> > > > - case PROP_REDIRECTING:
> > > > - old_val = priv->redirecting;
> > > > - priv->redirecting = g_value_get_boolean(value);
> > > > - if (old_val && !priv->redirecting) {
> > > > - /* This is a redirection completion case.
> > > > - Inject hotplug event in case we missed device changes
> > > > - during redirection processing. */
> > > > - handle_dev_change(self);
> > > > - }
> > > > - break;
> > > > - default:
> > > > - G_OBJECT_WARN_INVALID_PROPERTY_ID(gobject, prop_id, pspec);
> > > > - break;
> > > > - }
> > > > -}
> > > > -
> > > > -static void g_udev_client_class_init(GUdevClientClass *klass)
> > > > -{
> > > > - GObjectClass *gobject_class = G_OBJECT_CLASS(klass);
> > > > - GParamSpec *pspec;
> > > > -
> > > > - gobject_class->finalize = g_udev_client_finalize;
> > > > - gobject_class->get_property = g_udev_client_get_property;
> > > > - gobject_class->set_property = g_udev_client_set_property;
> > > > -
> > > > - signals[UEVENT_SIGNAL] =
> > > > - g_signal_new("uevent",
> > > > - G_OBJECT_CLASS_TYPE(klass),
> > > > - G_SIGNAL_RUN_FIRST,
> > > > - G_STRUCT_OFFSET(GUdevClientClass, uevent),
> > > > - NULL, NULL,
> > > > - g_cclosure_user_marshal_VOID__POINTER_BOOLEAN,
> > > > - G_TYPE_NONE,
> > > > - 2,
> > > > - G_TYPE_POINTER,
> > > > - G_TYPE_BOOLEAN);
> > > > -
> > > > - /**
> > > > - * GUdevClient::redirecting:
> > > > - *
> > > > - * This property indicates when a redirection operation
> > > > - * is in progress on a device. It's set back to FALSE
> > > > - * once the device is fully redirected to the guest.
> > > > - */
> > > > - pspec = g_param_spec_boolean("redirecting", "Redirecting",
> > > > - "USB redirection operation is in
> > > > progress",
> > > > - FALSE,
> > > > - G_PARAM_READWRITE |
> > > > G_PARAM_STATIC_STRINGS);
> > > > -
> > > > - g_object_class_install_property(gobject_class, PROP_REDIRECTING,
> > > > pspec);
> > > > -}
> > > > -
> > > > -/* comparing bus:addr and vid:pid */
> > > > -static gint compare_usb_devices(gconstpointer a, gconstpointer b)
> > > > -{
> > > > - const UsbDeviceInformation *a_info, *b_info;
> > > > - gboolean same_bus, same_addr, same_vid, same_pid;
> > > > - a_info = spice_usb_backend_device_get_info((SpiceUsbBackendDevice
> > > > *)a);
> > > > - b_info = spice_usb_backend_device_get_info((SpiceUsbBackendDevice
> > > > *)b);
> > > > -
> > > > -
> > > > - same_bus = (a_info->bus == b_info->bus);
> > > > - same_addr = (a_info->address == b_info->address);
> > > > - same_vid = (a_info->vid == b_info->vid);
> > > > - same_pid = (a_info->pid == b_info->pid);
> > > > -
> > > > - 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_usb_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 */
> > > > - SpiceUsbBackendDevice *temp = found->data;
> > > > - found->data = dev->data;
> > > > - dev->data = temp;
> > > > - }
> > > > - }
> > > > -
> > > > - 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,
> > > > - gboolean add)
> > > > -{
> > > > - GList *dev;
> > > > -
> > > > - for (dev = g_list_first(old_list); dev != NULL; dev =
> > > > g_list_next(dev)) {
> > > > - GList *found = g_list_find_custom(new_list, dev->data,
> > > > compare_usb_devices);
> > > > - if (found == NULL) {
> > > > - g_udev_device_print(dev->data, add ? "add" : "remove");
> > > > - g_signal_emit(self, signals[UEVENT_SIGNAL], 0, dev->data,
> > > > add);
> > > > - }
> > > > - }
> > > > -}
> > > > -
> > > > -static void handle_dev_change(GUdevClient *self)
> > > > -{
> > > > - GUdevClientPrivate *priv = self->priv;
> > > > - GError *err = NULL;
> > > > - GList *now_devs = NULL;
> > > > -
> > > > - if (priv->redirecting == TRUE) {
> > > > - /* On Windows, querying USB device list may return inconsistent
> > > > results
> > > > - if performed in parallel to redirection flow.
> > > > - A simulated hotplug event will be injected after redirection
> > > > - completion in order to process real device list changes that
> > > > may
> > > > - had taken place during redirection process. */
> > > > - return;
> > > > - }
> > > > -
> > > > - if(g_udev_client_list_devices(self, &now_devs, &err, __FUNCTION__) <
> > > > 0) {
> > > > - g_warning("could not retrieve device list");
> > > > - return;
> > > > - }
> > > > -
> > > > - 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:");
> > > > -
> > > > - /* Unregister devices that are not present anymore */
> > > > - notify_dev_state_change(self, priv->udev_list, now_devs, FALSE);
> > > > -
> > > > - /* Register newly inserted devices */
> > > > - notify_dev_state_change(self, now_devs, priv->udev_list, TRUE);
> > > > -
> > > > - /* keep most recent info: free previous list, and keep current list
> > > > */
> > > > - update_device_list(self, now_devs);
> > > > -}
> > > > -
> > > > -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);
> > > > -}
> > > > -
> > > > -#ifdef DEBUG_GUDEV_DEVICE_LISTS
> > > > -static void g_udev_device_print_list(GList *l, const gchar *msg)
> > > > -{
> > > > - GList *it;
> > > > -
> > > > - for (it = g_list_first(l); it != NULL; it=g_list_next(it)) {
> > > > - g_udev_device_print(it->data, msg);
> > > > - }
> > > > -}
> > > > -#endif
> > > > -
> > > > -static void g_udev_device_print(SpiceUsbBackendDevice *dev, const gchar
> > > > *msg)
> > > > -{
> > > > - const UsbDeviceInformation *info =
> > > > spice_usb_backend_device_get_info(dev);
> > > > -
> > > > - SPICE_DEBUG("%s: %d.%d 0x%04x:0x%04x class %d", msg,
> > > > - info->bus, info->address,
> > > > - info->vid, info->pid, info->class);
> > > > -}
> > > > diff --git a/src/win-usb-dev.h b/src/win-usb-dev.h
> > > > deleted file mode 100644
> > > > index fdfe73a..0000000
> > > > --- a/src/win-usb-dev.h
> > > > +++ /dev/null
> > > > @@ -1,84 +0,0 @@
> > > > -/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
> > > > -/*
> > > > - Copyright (C) 2012 Red Hat, Inc.
> > > > -
> > > > - Red Hat Authors:
> > > > - Arnon Gilboa <agilboa@xxxxxxxxxx>
> > > > - Uri Lublin <uril@xxxxxxxxxx>
> > > > -
> > > > - This library is free software; you can redistribute it and/or
> > > > - modify it under the terms of the GNU Lesser General Public
> > > > - License as published by the Free Software Foundation; either
> > > > - version 2.1 of the License, or (at your option) any later version.
> > > > -
> > > > - This library is distributed in the hope that it will be useful,
> > > > - but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > > - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > > > - Lesser General Public License for more details.
> > > > -
> > > > - You should have received a copy of the GNU Lesser General Public
> > > > - License along with this library; if not, see
> > > > <http://www.gnu.org/licenses/>.
> > > > -*/
> > > > -#ifndef __WIN_USB_DEV_H__
> > > > -#define __WIN_USB_DEV_H__
> > > > -
> > > > -#include <gio/gio.h>
> > > > -#include "usb-backend.h"
> > > > -
> > > > -G_BEGIN_DECLS
> > > > -
> > > > -/* GUdevClient */
> > > > -
> > > > -#define G_UDEV_TYPE_CLIENT (g_udev_client_get_type())
> > > > -#define G_UDEV_CLIENT(o) (G_TYPE_CHECK_INSTANCE_CAST((o),
> > > > G_UDEV_TYPE_CLIENT, GUdevClient))
> > > > -#define G_UDEV_CLIENT_CLASS(k) (G_TYPE_CHECK_CLASS_CAST((k),
> > > > G_UDEV_TYPE_CLIENT, GUdevClientClass))
> > > > -#define G_UDEV_IS_CLIENT(o) (G_TYPE_CHECK_INSTANCE_TYPE((o),
> > > > G_UDEV_TYPE_CLIENT))
> > > > -#define G_UDEV_IS_CLIENT_CLASS(k) (G_TYPE_CHECK_CLASS_TYPE((k),
> > > > G_UDEV_TYPE_CLIENT))
> > > > -#define G_UDEV_CLIENT_GET_CLASS(o) (G_TYPE_INSTANCE_GET_CLASS((o),
> > > > G_UDEV_TYPE_CLIENT, GUdevClientClass))
> > > > -
> > > > -typedef struct _GUdevClient GUdevClient;
> > > > -typedef struct _GUdevClientClass GUdevClientClass;
> > > > -typedef struct _GUdevClientPrivate GUdevClientPrivate;
> > > > -
> > > > -struct _GUdevClient
> > > > -{
> > > > - GObject parent;
> > > > -
> > > > - GUdevClientPrivate *priv;
> > > > -};
> > > > -
> > > > -struct _GUdevClientClass
> > > > -{
> > > > - GObjectClass parent_class;
> > > > -
> > > > - /* signals */
> > > > - void (*uevent)(GUdevClient *client, SpiceUsbBackendDevice *device,
> > > > gboolean add);
> > > > -};
> > > > -
> > > > -GType g_udev_client_get_type(void) G_GNUC_CONST;
> > > > -GUdevClient *g_udev_client_new(void);
> > > > -SpiceUsbBackend *g_udev_client_get_context(GUdevClient *client);
> > > > -void g_udev_client_report_devices(GUdevClient *client);
> > > > -
> > > > -GQuark g_udev_client_error_quark(void);
> > > > -#define G_UDEV_CLIENT_ERROR g_udev_client_error_quark()
> > > > -
> > > > -/**
> > > > - * GUdevClientError:
> > > > - * @G_UDEV_CLIENT_ERROR_FAILED: generic error code
> > > > - * @G_UDEV_CLIENT_LIBUSB_FAILED: a libusb call failed
> > > > - * @G_UDEV_CLIENT_WINAPI_FAILED: a winapi call failed
> > > > - *
> > > > - * Error codes returned by spice-client API.
> > > > - */
> > > > -typedef enum
> > > > -{
> > > > - G_UDEV_CLIENT_ERROR_FAILED = 1,
> > > > - G_UDEV_CLIENT_LIBUSB_FAILED,
> > > > - G_UDEV_CLIENT_WINAPI_FAILED
> > > > -} GUdevClientError;
> > > > -
> > > > -
> > > > -G_END_DECLS
> > > > -
> > > > -#endif /* __WIN_USB_DEV_H__ */
> > > > --
> > > > 2.17.1
> > > >
> > > > _______________________________________________
> > > > Spice-devel mailing list
> > > > Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> > > > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel