Re: [PATCH v4 1/5] net: Clarify early exit condition

[Vladimir Sementsov-Ogievskiy]

14.01.2021 01:10, Eric Blake wrote:
> On first glance, the loop in qmp_query_rx_filter() has early return
> paths that could leak any allocation of filter_list from a previous
> iteration.  But on closer inspection, it is obvious that all of the
> early exits are guarded by has_name, and that the bulk of the loop
> body can be executed at most once if the user is filtering by name,
> thus, any early exit coincides with an empty list.  Add asserts to
> make this obvious.

A bit simpler (for me :) observation:

But on closer inspection, it is obvious that all of the early exits are guarded 
by has_name, and in case when has_name is true we leave the loop (by break) 
immediately after allocation and assigning filter_list for the first time.

> Signed-off-by: Eric Blake <eblake@redhat.com>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

> ---
>   net/net.c | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/net/net.c b/net/net.c
> index e1035f21d183..e581c8a26868 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -1211,6 +1211,7 @@ RxFilterInfoList *qmp_query_rx_filter(bool has_name, 
> const char *name,
>           if (nc->info->type != NET_CLIENT_DRIVER_NIC) {
>               if (has_name) {
>                   error_setg(errp, "net client(%s) isn't a NIC", name);
> +                assert(!filter_list);
>                   return NULL;
>               }
>               continue;
> @@ -1236,6 +1237,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);
> +            assert(!filter_list);
>               return NULL;
>           }


--
Best regards,
Vladimir