Re: [PATCH v3 09/30] nbd/server: Clean up abuse of BlockExportOptionsNbd member @arg

[Eric Blake]

On Fri, Nov 04, 2022 at 05:06:51PM +0100, Markus Armbruster wrote:
> block-export-add argument @name defaults to the value of argument
> @node-name.
> 
> nbd_export_create() implements this by copying @node_name to @name.
> It leaves @has_node_name false, violating the "has_node_name ==
> !!node_name" invariant.  Unclean.  Falls apart when we elide
> @has_node_name (next commit): then QAPI frees the same value twice,
> once for @node_name and once @name.  iotest 307 duly explodes.
> 
> Goes back to commit c62d24e906 "blockdev-nbd: Boxed argument type for
> nbd-server-add" (v5.0.0).  Got moved from qmp_nbd_server_add() to
> nbd_export_create() (commit 56ee86261e), then copied back (commit
> b6076afcab).  Commit 8675cbd68b "nbd: Utilize QAPI_CLONE for type
> conversion" (v5.2.0) cleaned up the copy in qmp_nbd_server_add()
> noting
> 
>     Second, our assignment to arg->name is fishy: the generated QAPI code
>     for qapi_free_NbdServerAddOptions does not visit arg->name if
>     arg->has_name is false, but if it DID visit it, we would have
>     introduced a double-free situation when arg is finally freed.
> 
> Exactly.  However, the copy in nbd_export_create() remained dirty.
> 
> Clean it up.  Since the value stored in member @name is not actually
> used outside this function, use a local variable instead of modifying
> the QAPI object.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> Cc: Eric Blake <eblake@redhat.com>
> Cc: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> Cc: qemu-block@nongnu.org
> ---
>  nbd/server.c | 15 ++++++---------
>  1 file changed, 6 insertions(+), 9 deletions(-)
>

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

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