Eric Blake <ebl...@redhat.com> writes:
> If qmp_query_rx_filter() encounters an error on a second iteration, it
> leaks the memory from the first.
>
> Fixes: 9083da1d4c
> Signed-off-by: Eric Blake <ebl...@redhat.com>
> ---
> net/net.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/net/net.c b/net/net.c
> index 794c652282cb..eb65e110871a 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -1213,6 +1213,7 @@ RxFilterInfoList *qmp_query_rx_filter(bool has_name,
> const char *name,
NetClientState *nc;
RxFilterInfoList *filter_list = NULL, *last_entry = NULL;
QTAILQ_FOREACH(nc, &net_clients, next) {
RxFilterInfoList *entry;
RxFilterInfo *info;
if (has_name && strcmp(nc->name, name) != 0) {
continue;
}
If @has_name and we get here more than once, then multiple @net_clients
have the same name. How can that be? We are not supposed to return
multiple replies with the same @name, are we?
/* only query rx-filter information of NIC */
> if (nc->info->type != NET_CLIENT_DRIVER_NIC) {
> if (has_name) {
> error_setg(errp, "net client(%s) isn't a NIC", name);
> + qapi_free_RxFilterInfoList(filter_list);
Unless multiple @net_clients are named @name, @filter_list is null,
isn't it?
> return NULL;
> }
> continue;
}
/* only query information on queue 0 since the info is per nic,
* not per queue
*/
if (nc->queue_index != 0)
continue;
if (nc->info->query_rx_filter) {
info = nc->info->query_rx_filter(nc);
entry = g_malloc0(sizeof(*entry));
entry->value = info;
if (!filter_list) {
filter_list = entry;
>From now on, we must either return or free @filter_list.
} else {
last_entry->next = entry;
}
last_entry = entry;
> @@ -1238,6 +1239,7 @@ RxFilterInfoList *qmp_query_rx_filter(bool has_name,
> const char *name,
> } else if (has_name) {
> error_setg(errp, "net client(%s) doesn't support"
> " rx-filter querying", name);
> + qapi_free_RxFilterInfoList(filter_list);
Unless multiple @net_clients are named @name, @filter_list is null,
isn't it?
> return NULL;
> }
if (has_name) {
break;
}
}
I dislike this loop.
if (filter_list == NULL && has_name) {
error_setg(errp, "invalid net client name: %s", name);
}
return filter_list;
I should've strangled the optional @name parameter in the crib.
