Re: [PATCH v7 10/14] usbredir: Disconnect USB device asynchronously

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Sorry, I missed a few more things on this patch the last time I reviewed it :/

On Tue, 2016-03-08 at 16:05 +0200, Dmitry Fleytman wrote:
> From: Kirill Moizik <kmoizik@xxxxxxxxxx>
> 
> Signed-off-by: Kirill Moizik <kmoizik@xxxxxxxxxx>
> Signed-off-by: Dmitry Fleytman <dfleytma@xxxxxxxxxx>
> ---
>  src/channel-usbredir.c | 48 +++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 41 insertions(+), 7 deletions(-)
> 
> diff --git a/src/channel-usbredir.c b/src/channel-usbredir.c
> index 7253ce6..ba9e4ab 100644
> --- a/src/channel-usbredir.c
> +++ b/src/channel-usbredir.c
> @@ -113,20 +113,54 @@ static void
> spice_usbredir_channel_init(SpiceUsbredirChannel *channel)
>  }
>  
>  #ifdef USE_USBREDIR
> +
> +static void _channel_reset_finish(SpiceUsbredirChannel *channel)
> +{
> +    SpiceUsbredirChannelPrivate *priv = channel->priv;
> +
> +    spice_usbredir_channel_lock(channel);
> +
> +    usbredirhost_close(priv->host);
> +    priv->host = NULL;
> +
> +    /* Call set_context to re-create the host */
> +    spice_usbredir_channel_set_context(channel, priv->context);
> +
> +    spice_usbredir_channel_unlock(channel);
> +}
> +
> +static void _channel_reset_cb(GObject *gobject,
> +                              GAsyncResult *result,
> +                              gpointer user_data)
> +{
> +    SpiceChannel *spice_channel =  SPICE_CHANNEL(gobject);
> +    SpiceUsbredirChannel *channel = SPICE_USBREDIR_CHANNEL(spice_channel);
> +    gboolean migrating = GPOINTER_TO_UINT(user_data);
> +    GError *err = NULL;
> +
> +    _channel_reset_finish(channel);
> +
> +    SPICE_CHANNEL_CLASS(spice_usbredir_channel_parent_class)
> ->channel_reset(spice_channel, migrating);
> +
> +    spice_usbredir_channel_disconnect_device_finish(channel, result, &err);

err is leaked if this call is returns a failure.

> +    g_object_unref(result);

result should not be unreffed here. The only reference should be held by the
async operation that invokes this callback. Presumably it will be released after
this function returns and you will get a crash. If there's no crash, there may
be a leak, but it should be fixed elsewhere.

> +}
> +
>  static void spice_usbredir_channel_reset(SpiceChannel *c, gboolean migrating)
>  {
>      SpiceUsbredirChannel *channel = SPICE_USBREDIR_CHANNEL(c);
>      SpiceUsbredirChannelPrivate *priv = channel->priv;
>  
>      if (priv->host) {
> -        if (priv->state == STATE_CONNECTED)
> -            spice_usbredir_channel_disconnect_device(channel);
> -        usbredirhost_close(priv->host);
> -        priv->host = NULL;
> -        /* Call set_context to re-create the host */
> -        spice_usbredir_channel_set_context(channel, priv->context);
> +        if (priv->state == STATE_CONNECTED) {
> +            spice_usbredir_channel_disconnect_device_async(channel, NULL,
> +                _channel_reset_cb, GUINT_TO_POINTER(migrating));
> +        } else {
> +            _channel_reset_finish(channel);
> +        }
> +    } else {
> +        SPICE_CHANNEL_CLASS(spice_usbredir_channel_parent_class)
> ->channel_reset(c, migrating);
>      }
> -    SPICE_CHANNEL_CLASS(spice_usbredir_channel_parent_class)
> ->channel_reset(c, migrating);
>  }
>  #endif
>  


Reviewed-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]