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 <arm...@redhat.com> > Cc: Eric Blake <ebl...@redhat.com> > Cc: Vladimir Sementsov-Ogievskiy <vsement...@yandex-team.ru> > Cc: qemu-bl...@nongnu.org > --- > nbd/server.c | 15 ++++++--------- > 1 file changed, 6 insertions(+), 9 deletions(-) >
Reviewed-by: Eric Blake <ebl...@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
