On Thu, 2015-10-29 at 17:26 +0200, Dmitry Fleytman wrote:
> Signed-off-by: Dmitry Fleytman <dmitry@xxxxxxxxxx>
> ---
> src/usb-device-manager.c | 50 ++++++++++++++++++++++++++++++-----------------
> -
> 1 file changed, 31 insertions(+), 19 deletions(-)
>
> diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c
> index d5fec8f..53820b4 100644
> --- a/src/usb-device-manager.c
> +++ b/src/usb-device-manager.c
> @@ -230,7 +230,7 @@ static void
> spice_usb_device_manager_init(SpiceUsbDeviceManager *self)
> priv = SPICE_USB_DEVICE_MANAGER_GET_PRIVATE(self);
> self->priv = priv;
>
> -#ifdef G_OS_WIN32
> +#if defined(G_OS_WIN32) && defined(USE_USBREDIR)
> priv->use_usbclerk = TRUE;
> #endif
This particular change seems a bit unrelated to this commit. It probably should
be moved to the patch where priv->use_usbclerk was introduced, no?
> priv->channels = g_ptr_array_new();
> @@ -255,10 +255,12 @@ static gboolean
> spice_usb_device_manager_initable_init(GInitable *initable,
> #endif
>
> #ifdef G_OS_WIN32
> - priv->installer = spice_win_usb_driver_new(err);
> - if (!priv->installer) {
> - SPICE_DEBUG("failed to initialize winusb driver");
> - return FALSE;
> + if (priv->use_usbclerk) {
> + priv->installer = spice_win_usb_driver_new(err);
> + if (!priv->installer) {
> + SPICE_DEBUG("failed to initialize winusb driver");
> + return FALSE;
> + }
> }
> #endif
>
> @@ -365,15 +367,17 @@ static void spice_usb_device_manager_finalize(GObject
> *gobject)
> free(priv->auto_conn_filter_rules);
> free(priv->redirect_on_connect_rules);
> #ifdef G_OS_WIN32
> - if (priv->installer)
> + if (priv->installer) {
> + g_warn_if_fail(priv->use_usbclerk);
> g_object_unref(priv->installer);
> + }
> #endif
> #endif
>
> g_free(priv->auto_connect_filter);
> g_free(priv->redirect_on_connect);
>
> -#ifdef G_OS_WIN32
> +#if defined(G_OS_WIN32) && defined(USE_USBREDIR)
As I mentioned in the review for patch 8, this change shouldn be done earlier in
an earlier patch. it's not directly related to this patch.
> if (!priv->use_usbclerk) {
> if(priv->auto_connect)
> _usbdk_autoredir_disable(self);
> @@ -430,7 +434,7 @@ static void spice_usb_device_manager_set_property(GObject
> *gobject,
> break;
> case PROP_AUTO_CONNECT:
> priv->auto_connect = g_value_get_boolean(value);
> -#ifdef G_OS_WIN32
> +#if defined(G_OS_WIN32) && defined(USE_USBREDIR)
Same as above. Consider moving to an earlier patch if the change is needed.
> if (!priv->use_usbclerk) {
> if (priv->auto_connect) {
> _usbdk_autoredir_enable(self);
> @@ -935,12 +939,14 @@ static void
> spice_usb_device_manager_remove_dev(SpiceUsbDeviceManager *self,
> }
>
> #ifdef G_OS_WIN32
> - const guint8 state = spice_usb_device_get_state(device);
> - if ((state == SPICE_USB_DEVICE_STATE_INSTALLING) ||
> - (state == SPICE_USB_DEVICE_STATE_UNINSTALLING)) {
> - SPICE_DEBUG("skipping " DEV_ID_FMT ". It is un/installing its
> driver",
> - bus, address);
> - return;
> + if (priv->use_usbclerk) {
> + const guint8 state = spice_usb_device_get_state(device);
> + if ((state == SPICE_USB_DEVICE_STATE_INSTALLING) ||
> + (state == SPICE_USB_DEVICE_STATE_UNINSTALLING)) {
> + SPICE_DEBUG("skipping " DEV_ID_FMT ". It is un/installing its
> driver",
> + bus, address);
> + return;
> + }
> }
> #endif
>
> @@ -1141,6 +1147,7 @@ static void
> spice_usb_device_manager_drv_install_cb(GObject *gobject,
> g_free(cbinfo);
>
> g_return_if_fail(SPICE_IS_USB_DEVICE_MANAGER(self));
> + g_return_if_fail(self->priv->use_usbclerk);
> g_return_if_fail(SPICE_IS_WIN_USB_DRIVER(installer));
> g_return_if_fail(device!= NULL);
>
> @@ -1173,6 +1180,7 @@ static void
> spice_usb_device_manager_drv_uninstall_cb(GObject *gobject,
>
> SPICE_DEBUG("Win USB driver uninstall finished");
> g_return_if_fail(SPICE_IS_USB_DEVICE_MANAGER(self));
> + g_return_if_fail(self->priv->use_usbclerk);
Minor issue since these statements are only executed on a programming error, but
returning early here will leak cbinfo.
>
> if (!spice_win_usb_driver_uninstall_finish(cbinfo->installer, res, &err))
> {
> g_warning("win usb driver uninstall failed -- %s", err->message);
> @@ -1576,15 +1584,18 @@ void
> spice_usb_device_manager_connect_device_async(SpiceUsbDeviceManager *self,
> {
>
> #if defined(USE_USBREDIR) && defined(G_OS_WIN32)
> - _spice_usb_device_manager_install_driver_async(self, device, cancellable,
> - callback, user_data);
> -#else
> + if (self->priv->use_usbclerk) {
> + _spice_usb_device_manager_install_driver_async(self, device,
> cancellable,
> + callback, user_data);
> + return;
> + }
> +#endif
> +
> _spice_usb_device_manager_connect_device_async(self,
> device,
> cancellable,
> callback,
> user_data);
> -#endif
> }
>
> /**
> @@ -1637,7 +1648,8 @@ void
> spice_usb_device_manager_disconnect_device(SpiceUsbDeviceManager *self,
> spice_usbredir_channel_disconnect_device(channel);
>
> #ifdef G_OS_WIN32
> - _spice_usb_device_manager_uninstall_driver_async(self, device);
> + if(self->priv->use_usbclerk)
> + _spice_usb_device_manager_uninstall_driver_async(self, device);
> #endif
>
> #endif
Reviewed-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel