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

[Jason Wang <jasowang@redhat.com>]

在 2021/3/17 上午5:27, 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 ?


Yes, stored->vhostfds should be used here.

Alexey, please send patch to fix this.

Thanks


>
> thanks
> -- PMM
>