[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 09/30] nbd/server: Clean up abuse of BlockExportOptionsNbd
From: |
Eric Blake |
Subject: |
Re: [PATCH v3 09/30] nbd/server: Clean up abuse of BlockExportOptionsNbd member @arg |
Date: |
Fri, 4 Nov 2022 14:35:02 -0500 |
User-agent: |
NeoMutt/20220429 |
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
- Re: [PATCH v3 13/30] qapi dump: Elide redundant has_FOO in generated C, (continued)
- [PATCH v3 17/30] qapi misc: Elide redundant has_FOO in generated C, Markus Armbruster, 2022/11/04
- [PATCH v3 18/30] qapi net: Elide redundant has_FOO in generated C, Markus Armbruster, 2022/11/04
- [PATCH v3 07/30] qapi audio: Elide redundant has_FOO in generated C, Markus Armbruster, 2022/11/04
- [PATCH v3 15/30] qapi machine: Elide redundant has_FOO in generated C, Markus Armbruster, 2022/11/04
- [PATCH v3 20/30] qapi qdev qom: Elide redundant has_FOO in generated C, Markus Armbruster, 2022/11/04
- [PATCH v3 11/30] qapi chardev: Elide redundant has_FOO in generated C, Markus Armbruster, 2022/11/04
- [PATCH v3 09/30] nbd/server: Clean up abuse of BlockExportOptionsNbd member @arg, Markus Armbruster, 2022/11/04
- [PATCH v3 02/30] qapi: Tidy up whitespace in generated code, Markus Armbruster, 2022/11/04
- [PATCH v3 29/30] qapi qga: Elide redundant has_FOO in generated C, Markus Armbruster, 2022/11/04
- [PATCH v3 22/30] qapi rocker: Elide redundant has_FOO in generated C, Markus Armbruster, 2022/11/04
- [PATCH v3 14/30] qapi job: Elide redundant has_FOO in generated C, Markus Armbruster, 2022/11/04
- Re: [PATCH v3 00/30] qapi: Elide redundant has_FOO in generated C, Markus Armbruster, 2022/11/04