Re: [PATCH v1 03/32] util: netdevbridge: use VIR_AUTOFREE instead of VIR_FREE for scalar types

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, Jul 28, 2018 at 11:31:18PM +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@xxxxxxxxx>
> ---
>  src/util/virnetdevbridge.c | 45 +++++++++++++++------------------------------
>  1 file changed, 15 insertions(+), 30 deletions(-)
>
> diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c
> index e46ac35..bf30d7c 100644
> --- a/src/util/virnetdevbridge.c
> +++ b/src/util/virnetdevbridge.c
> @@ -126,8 +126,7 @@ static int virNetDevBridgeSet(const char *brname,
>                                int fd,                 /* control socket */
>                                struct ifreq *ifr)      /* pre-filled bridge name */
>  {
> -    char *path = NULL;
> -    int ret = -1;
> +    VIR_AUTOFREE(char *) path = NULL;
>
>      if (virAsprintf(&path, SYSFS_NET_DIR "%s/bridge/%s", brname, paramname) < 0)
>          return -1;
> @@ -138,7 +137,7 @@ static int virNetDevBridgeSet(const char *brname,
>          if (virFileWriteStr(path, valuestr, 0) < 0) {
>              virReportSystemError(errno,
>                                   _("Unable to set bridge %s %s"), brname, paramname);
> -            goto cleanup;
> +            return -1;
>          }
>      } else {
>          unsigned long paramid;
> @@ -149,21 +148,18 @@ static int virNetDevBridgeSet(const char *brname,
>          } else {
>              virReportSystemError(EINVAL,
>                                   _("Unable to set bridge %s %s"), brname, paramname);
> -            goto cleanup;
> +            return -1;
>          }
>          unsigned long args[] = { paramid, value, 0, 0 };
>          ifr->ifr_data = (char*)&args;
>          if (ioctl(fd, SIOCDEVPRIVATE, ifr) < 0) {
>              virReportSystemError(errno,
>                                   _("Unable to set bridge %s %s"), brname, paramname);
> -            goto cleanup;
> +            return -1;
>          }
>      }
>
> -    ret = 0;
> - cleanup:
> -    VIR_FREE(path);
> -    return ret;
> +    return 0;
>  }
>
>
> @@ -171,7 +167,7 @@ static int virNetDevBridgeGet(const char *brname,
>                                const char *paramname,  /* sysfs param name */
>                                unsigned long *value)   /* current value */
>  {
> -    char *path = NULL;
> +    VIR_AUTOFREE(char *) path = NULL;

Referring to my response to previous patch, I'll move ^this at the end of the
"declare" block (there are a few identical spots across the patch).

...

>   malformed_resp:
> @@ -1069,7 +1055,7 @@ virNetDevBridgeFDBAddDel(const virMacAddr *mac, const char *ifname,
>                           unsigned int flags, bool isAdd)
>  {
>      int ret = -1;
> -    struct nlmsghdr *resp = NULL;
> +    VIR_AUTOFREE(struct nlmsghdr *) resp = NULL;
>      struct nlmsgerr *err;
>      unsigned int recvbuflen;
>      struct nl_msg *nl_msg;

So, I believe ^this external type can easily be turned into an autoclean
variant.

> @@ -1142,7 +1128,6 @@ virNetDevBridgeFDBAddDel(const virMacAddr *mac, const char *ifname,
>      ret = 0;
>   cleanup:
>      nlmsg_free(nl_msg);

So that ^this would be done automatically.

Otherwise it the patch looks fine.

Erik

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



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux