[PATCH v3 1/7] net: Clarify early exit condition

Eric Blake posted 7 patches 4 years, 9 months ago
There is a newer version of this series
[PATCH v3 1/7] net: Clarify early exit condition
Posted by Eric Blake 4 years, 9 months ago
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.

Signed-off-by: Eric Blake <eblake@redhat.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;
         }

-- 
2.29.2


Re: [PATCH v3 1/7] net: Clarify early exit condition
Posted by Markus Armbruster 4 years, 8 months ago
Eric Blake <eblake@redhat.com> writes:

> 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.
>
> Signed-off-by: Eric Blake <eblake@redhat.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;
>          }

Reviewed-by: Markus Armbruster <armbru@redhat.com>