qemu-devel
[Top][All Lists]
Advanced

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

Re: [PULL 28/34] nbd/client-connection: return only one io channel


From: Eric Blake
Subject: Re: [PULL 28/34] nbd/client-connection: return only one io channel
Date: Fri, 18 Jun 2021 10:55:29 -0500
User-agent: NeoMutt/20210205

On Thu, Jun 17, 2021 at 09:32:02PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Logic is wrong and uninitialized use of ioc really possible, as Peter (and 
> clang) reports.
> 
> So, I propose the following squash-in. It doesn't conflict with following 
> patches.
> 
> squash-in:
> 
> diff --git a/nbd/client-connection.c b/nbd/client-connection.c
> index 500b8591e8..396d7f17f0 100644
> --- a/nbd/client-connection.c
> +++ b/nbd/client-connection.c
> @@ -275,7 +275,6 @@ QIOChannel *coroutine_fn
>  nbd_co_establish_connection(NBDClientConnection *conn, NBDExportInfo *info,
>                              Error **errp)
>  {
> -    QIOChannel *ioc;
>      QemuThread thread;
>      if (conn->do_negotiation) {
> @@ -293,17 +292,19 @@ nbd_co_establish_connection(NBDClientConnection *conn, 
> NBDExportInfo *info,
>              if (conn->sioc) {
>                  /* Previous attempt finally succeeded in background */
>                  if (conn->do_negotiation) {
> -                    ioc = g_steal_pointer(&conn->ioc);
>                      memcpy(info, &conn->updated_info, sizeof(*info));
> +                    if (conn->ioc) {
> +                        /* TLS channel now has own reference to parent */
> +                        object_unref(OBJECT(conn->sioc));
> +                        conn->sioc = NULL;
> +
> +                        return g_steal_pointer(&conn->ioc);
> +                    }
>                  }
> -                if (ioc) {
> -                    /* TLS channel now has own reference to parent */
> -                    object_unref(OBJECT(conn->sioc));
> -                } else {
> -                    ioc = QIO_CHANNEL(conn->sioc);
> -                }
> -                conn->sioc = NULL;
> -                return ioc;
> +
> +                assert(!conn->ioc);
> +
> +                return QIO_CHANNEL(g_steal_pointer(&conn->sioc));

Reviewed-by: Eric Blake <eblake@redhat.com>

I'll squash this in and send v2 of the pull request

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org




reply via email to

[Prev in Thread] Current Thread [Next in Thread]