Re: [PULL V2 16/20] qapi: net: Add query-netdev command

[Peter Maydell]

On Mon, 15 Mar 2021 at 09:15, Jason Wang <jasowang@redhat.com> wrote:
>
> From: Alexey Kirillov <lekiravi@yandex-team.ru>
>
> The query-netdev command is used to get the configuration of the current
> network device backends (netdevs).
> This is the QMP analog of the HMP command "info network" but only for
> netdevs (i.e. excluding NIC and hubports).
>
> The query-netdev command returns an array of objects of the NetdevInfo
> type, which are an extension of Netdev type. It means that response can
> be used for netdev-add after small modification. This can be useful for
> recreate the same netdev configuration.
>
> Information about the network device is filled in when it is created or
> modified and is available through the NetClientState->stored_config.
>
> Signed-off-by: Alexey Kirillov <lekiravi@yandex-team.ru>
> Acked-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>

Hi; Coverity is doubtful (CID 1450841) about this code:

> @@ -668,12 +686,65 @@ static void net_init_tap_one(const NetdevTapOptions 
> *tap, NetClientState *peer,

> +        if (!stored->has_fds) {
> +            stored->has_fds = true;
> +            stored->fds = g_strdup_printf("%d", fd);
> +        } else {
> +            char *tmp_s = stored->fds;
> +            stored->fds = g_strdup_printf("%s:%d", stored->fds, fd);
> +            g_free(tmp_s);
> +        }

Here we have a bit of code which maintains stored->fds as a
colon-separated string of integers, by tacking the new fd onto
the end of the old string if it's already present.

> @@ -731,6 +813,15 @@ static void net_init_tap_one(const NetdevTapOptions 
> *tap, NetClientState *peer,
>          }
>          options.opaque = (void *)(uintptr_t)vhostfd;
>
> +        if (!stored->has_vhostfds) {
> +            stored->has_vhostfds = true;
> +            stored->vhostfds = g_strdup_printf("%d", vhostfd);
> +        } else {
> +            char *tmp_s = stored->vhostfds;
> +            stored->vhostfds = g_strdup_printf("%s:%d", stored->fds, 
> vhostfd);
> +            g_free(tmp_s);
> +        }

Here we have a bit of code that's kind of similar, except that
the first argument to g_strdup_printf() is 'stored->fds', not
'stored->vhostfds'.

Coverity suspects cut-n-paste error -- is it right ?

thanks
-- PMM