[libvirt] [PATCH v3 30/35] util: arptable: use VIR_AUTOFREE instead of VIR_FREE for scalar types

Sukrit Bhatnagar posted 35 patches 6 years, 10 months ago
There is a newer version of this series
[libvirt] [PATCH v3 30/35] util: arptable: use VIR_AUTOFREE instead of VIR_FREE for scalar types
Posted by Sukrit Bhatnagar 6 years, 10 months ago
By making use of GNU C's cleanup attribute handled by the
VIR_AUTOFREE macro for declaring scalar variables, majority
of the VIR_FREE calls can be dropped, which in turn leads to
getting rid of most of our cleanup sections.

Signed-off-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com>
---
 src/util/virarptable.c | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/src/util/virarptable.c b/src/util/virarptable.c
index c0e90dc..bf40267 100644
--- a/src/util/virarptable.c
+++ b/src/util/virarptable.c
@@ -71,9 +71,8 @@ virArpTableGet(void)
 {
     int num = 0;
     int msglen;
-    void *nlData = NULL;
+    VIR_AUTOFREE(void *) nlData = NULL;
     virArpTablePtr table = NULL;
-    char *ipstr = NULL;
     struct nlmsghdr* nh;
     struct rtattr * tb[NDA_MAX+1];
 
@@ -89,6 +88,7 @@ virArpTableGet(void)
     VIR_WARNINGS_NO_CAST_ALIGN
     for (; NLMSG_OK(nh, msglen); nh = NLMSG_NEXT(nh, msglen)) {
         VIR_WARNINGS_RESET
+        VIR_AUTOFREE(char *) ipstr = NULL;
         struct ndmsg *r = NLMSG_DATA(nh);
         int len = nh->nlmsg_len;
         void *addr;
@@ -108,7 +108,7 @@ virArpTableGet(void)
             continue;
 
         if (nh->nlmsg_type == NLMSG_DONE)
-            goto end_of_netlink_messages;
+            return table;
 
         VIR_WARNINGS_NO_CAST_ALIGN
         parse_rtattr(tb, NDA_MAX, NDA_RTA(r),
@@ -134,8 +134,6 @@ virArpTableGet(void)
 
             if (VIR_STRDUP(table->t[num].ipaddr, ipstr) < 0)
                 goto cleanup;
-
-            VIR_FREE(ipstr);
         }
 
         if (tb[NDA_LLADDR]) {
@@ -154,14 +152,8 @@ virArpTableGet(void)
         }
     }
 
- end_of_netlink_messages:
-    VIR_FREE(nlData);
-    return table;
-
  cleanup:
     virArpTableFree(table);
-    VIR_FREE(ipstr);
-    VIR_FREE(nlData);
     return NULL;
 }
 
-- 
1.8.3.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 30/35] util: arptable: use VIR_AUTOFREE instead of VIR_FREE for scalar types
Posted by Erik Skultety 6 years, 10 months ago
On Sat, Jun 30, 2018 at 02:30:34PM +0530, Sukrit Bhatnagar wrote:
> By making use of GNU C's cleanup attribute handled by the
> VIR_AUTOFREE macro for declaring scalar variables, majority
> of the VIR_FREE calls can be dropped, which in turn leads to
> getting rid of most of our cleanup sections.
>
> Signed-off-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com>
> ---
>  src/util/virarptable.c | 14 +++-----------
>  1 file changed, 3 insertions(+), 11 deletions(-)
>
> diff --git a/src/util/virarptable.c b/src/util/virarptable.c
> index c0e90dc..bf40267 100644
> --- a/src/util/virarptable.c
> +++ b/src/util/virarptable.c
> @@ -71,9 +71,8 @@ virArpTableGet(void)
>  {
>      int num = 0;
>      int msglen;
> -    void *nlData = NULL;
> +    VIR_AUTOFREE(void *) nlData = NULL;
>      virArpTablePtr table = NULL;
> -    char *ipstr = NULL;
>      struct nlmsghdr* nh;
>      struct rtattr * tb[NDA_MAX+1];
>
> @@ -89,6 +88,7 @@ virArpTableGet(void)
>      VIR_WARNINGS_NO_CAST_ALIGN
>      for (; NLMSG_OK(nh, msglen); nh = NLMSG_NEXT(nh, msglen)) {
>          VIR_WARNINGS_RESET
> +        VIR_AUTOFREE(char *) ipstr = NULL;

okay, but see below, ^this declaration can be moved further down

>          struct ndmsg *r = NLMSG_DATA(nh);
>          int len = nh->nlmsg_len;
>          void *addr;
> @@ -108,7 +108,7 @@ virArpTableGet(void)
>              continue;
>
>          if (nh->nlmsg_type == NLMSG_DONE)
> -            goto end_of_netlink_messages;
> +            return table;
>
>          VIR_WARNINGS_NO_CAST_ALIGN
>          parse_rtattr(tb, NDA_MAX, NDA_RTA(r),
> @@ -134,8 +134,6 @@ virArpTableGet(void)
>
>              if (VIR_STRDUP(table->t[num].ipaddr, ipstr) < 0)
>                  goto cleanup;
> -
> -            VIR_FREE(ipstr);

ipstr is only used in ^this corresponding block, so you can define it in ^this
corresponding rather than the 'for' block.

With that:

Reviewed-by: Erik Skultety <eskultet@redhat.com>

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list