[libvirt] [PATCH 4/6] util: netdev: use VIR_AUTOCLOSE instead of VIR_FORCE_CLOSE

Erik Skultety eskultet at redhat.com
Tue Sep 11 12:44:37 UTC 2018


On Mon, Sep 10, 2018 at 11:47:53AM +0800, Shi Lei wrote:
> By making use of GNU C's cleanup attribute handled by the VIR_AUTOCLOSE macro,
> many of the VIR_FORCE_CLOSE calls can be dropped, which in turn leads to
> getting rid of many of our cleanup sections.
>
> Signed-off-by: Shi Lei <shi_lei at massclouds.com>
> ---
>  src/util/virnetdev.c | 253 +++++++++++++++----------------------------
>  1 file changed, 87 insertions(+), 166 deletions(-)
>
> diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
> index 5d4ad24..7f43f15 100644
> --- a/src/util/virnetdev.c
> +++ b/src/util/virnetdev.c
> @@ -200,27 +200,22 @@ virNetDevSetupControl(const char *ifname ATTRIBUTE_UNUSED,
>   */
>  int virNetDevExists(const char *ifname)
>  {
> -    int fd = -1;
> -    int ret = -1;
>      struct ifreq ifr;
> +    VIR_AUTOCLOSE(fd);
>
>      if ((fd = virNetDevSetupControl(ifname, &ifr)) < 0)
>          return -1;
>
>      if (ioctl(fd, SIOCGIFFLAGS, &ifr)) {
>          if (errno == ENODEV || errno == ENXIO)
> -            ret = 0;
> -        else
> -            virReportSystemError(errno,
> -                                 _("Unable to check interface flags for %s"), ifname);
> -        goto cleanup;
> -    }
> +            return 0;
>
> -    ret = 1;
> +        virReportSystemError(errno, _("Unable to check interface flags for %s"),
> +                             ifname);
> +        return -1;
> +    }
>
> - cleanup:
> -    VIR_FORCE_CLOSE(fd);
> -    return ret;
> +    return 1;
>  }
>  #else
>  int virNetDevExists(const char *ifname)
> @@ -251,20 +246,20 @@ virNetDevSetMACInternal(const char *ifname,
>                          const virMacAddr *macaddr,
>                          bool quiet)
>  {
> -    int fd = -1;
> -    int ret = -1;
>      struct ifreq ifr;
>      char macstr[VIR_MAC_STRING_BUFLEN];
> +    VIR_AUTOCLOSE(fd);
>
>      if ((fd = virNetDevSetupControl(ifname, &ifr)) < 0)
>          return -1;
>
>      /* To fill ifr.ifr_hdaddr.sa_family field */
>      if (ioctl(fd, SIOCGIFHWADDR, &ifr) < 0) {
> -        virReportSystemError(errno,
> -                             _("Cannot get interface MAC on '%s'"),
> +        virReportSystemError(errno, _("Cannot get interface MAC on '%s'"),
>                               ifname);
> -        goto cleanup;
> +
> +        VIR_DEBUG("SIOCSIFHWADDR %s get MAC - Fail", ifname);
> +        return -1;
>      }
>
>      virMacAddrGetRaw(macaddr, (unsigned char *)ifr.ifr_hwaddr.sa_data);
> @@ -272,24 +267,22 @@ virNetDevSetMACInternal(const char *ifname,
>      if (ioctl(fd, SIOCSIFHWADDR, &ifr) < 0) {
>
>          if (quiet &&
> -            (errno == EADDRNOTAVAIL || errno == EPERM))
> -            goto cleanup;
> +            (errno == EADDRNOTAVAIL || errno == EPERM)) {
> +            VIR_DEBUG("SIOCSIFHWADDR %s MAC=%s - Fail",
> +                      ifname, virMacAddrFormat(macaddr, macstr));
> +            return -1;
> +        }
>
>          virReportSystemError(errno,
>                               _("Cannot set interface MAC to %s on '%s'"),
>                               virMacAddrFormat(macaddr, macstr), ifname);
> -        goto cleanup;
> +        return -1;
>      }
>
> -    ret = 0;
> +    VIR_DEBUG("SIOCSIFHWADDR %s MAC=%s - Success",
> +              ifname, virMacAddrFormat(macaddr, macstr));
>
> - cleanup:
> -    VIR_DEBUG("SIOCSIFHWADDR %s MAC=%s - %s",
> -              ifname, virMacAddrFormat(macaddr, macstr),
> -              ret < 0 ? "Fail" : "Success");
> -
> -    VIR_FORCE_CLOSE(fd);
> -    return ret;
> +    return 0;
>  }
>
>
> @@ -305,8 +298,7 @@ virNetDevSetMACInternal(const char *ifname,
>          struct ifreq ifr;
>          struct sockaddr_dl sdl;
>          char mac[VIR_MAC_STRING_BUFLEN + 1] = ":";
> -        int s;
> -        int ret = -1;
> +        VIR_AUTOCLOSE(s);
>
>          if ((s = virNetDevSetupControl(ifname, &ifr)) < 0)
>              return -1;
> @@ -320,23 +312,19 @@ virNetDevSetMACInternal(const char *ifname,
>
>          if (ioctl(s, SIOCSIFLLADDR, &ifr) < 0) {
>              if (quiet &&
> -                (errno == EADDRNOTAVAIL || errno == EPERM))
> -                goto cleanup;
> +                (errno == EADDRNOTAVAIL || errno == EPERM)) {
> +                VIR_DEBUG("SIOCSIFLLADDR %s MAC=%s - Fail", ifname, mac + 1);
> +                return -1;
> +            }
>
>              virReportSystemError(errno,
>                                   _("Cannot set interface MAC to %s on '%s'"),
>                                   mac + 1, ifname);
> -            goto cleanup;
> +            return -1;
>          }
>
> -        ret = 0;
> - cleanup:
> -        VIR_DEBUG("SIOCSIFLLADDR %s MAC=%s - %s", ifname, mac + 1,
> -                  ret < 0 ? "Fail" : "Success");
> -
> -        VIR_FORCE_CLOSE(s);
> -
> -        return ret;
> +        VIR_DEBUG("SIOCSIFLLADDR %s MAC=%s - Success", ifname, mac + 1);
> +        return 0;
>  }
>
>
> @@ -379,9 +367,8 @@ virNetDevSetMAC(const char *ifname,
>  int virNetDevGetMAC(const char *ifname,
>                      virMacAddrPtr macaddr)
>  {
> -    int fd = -1;
> -    int ret = -1;
>      struct ifreq ifr;
> +    VIR_AUTOCLOSE(fd);
>
>      if ((fd = virNetDevSetupControl(ifname, &ifr)) < 0)
>          return -1;
> @@ -390,16 +377,12 @@ int virNetDevGetMAC(const char *ifname,
>          virReportSystemError(errno,
>                               _("Cannot get interface MAC on '%s'"),
>                               ifname);
> -        goto cleanup;
> +        return -1;
>      }
>
>      virMacAddrSetRaw(macaddr, (unsigned char *)ifr.ifr_hwaddr.sa_data);
>
> -    ret = 0;
> -
> - cleanup:
> -    VIR_FORCE_CLOSE(fd);
> -    return ret;
> +    return 0;
>  }
>  #else
>  int virNetDevGetMAC(const char *ifname,
> @@ -424,9 +407,8 @@ int virNetDevGetMAC(const char *ifname,
>   */
>  int virNetDevGetMTU(const char *ifname)
>  {
> -    int fd = -1;
> -    int ret = -1;
>      struct ifreq ifr;
> +    VIR_AUTOCLOSE(fd);
>
>      if ((fd = virNetDevSetupControl(ifname, &ifr)) < 0)
>          return -1;
> @@ -435,14 +417,10 @@ int virNetDevGetMTU(const char *ifname)
>          virReportSystemError(errno,
>                               _("Cannot get interface MTU on '%s'"),
>                               ifname);
> -        goto cleanup;
> +        return -1;
>      }
>
> -    ret = ifr.ifr_mtu;
> -
> - cleanup:
> -    VIR_FORCE_CLOSE(fd);
> -    return ret;
> +    return ifr.ifr_mtu;
>  }
>  #else
>  int virNetDevGetMTU(const char *ifname)
> @@ -468,9 +446,8 @@ int virNetDevGetMTU(const char *ifname)
>   */
>  int virNetDevSetMTU(const char *ifname, int mtu)
>  {
> -    int fd = -1;
> -    int ret = -1;
>      struct ifreq ifr;
> +    VIR_AUTOCLOSE(fd);
>
>      if ((fd = virNetDevSetupControl(ifname, &ifr)) < 0)
>          return -1;
> @@ -481,14 +458,10 @@ int virNetDevSetMTU(const char *ifname, int mtu)
>          virReportSystemError(errno,
>                               _("Cannot set interface MTU on '%s'"),
>                               ifname);
> -        goto cleanup;
> +        return -1;
>      }
>
> -    ret = 0;
> -
> - cleanup:
> -    VIR_FORCE_CLOSE(fd);
> -    return ret;
> +    return 0;
>  }
>  #else
>  int virNetDevSetMTU(const char *ifname, int mtu ATTRIBUTE_UNUSED)
> @@ -592,9 +565,8 @@ int virNetDevSetNamespace(const char *ifname, pid_t pidInNs)
>   */
>  int virNetDevSetName(const char* ifname, const char *newifname)
>  {
> -    int fd = -1;
> -    int ret = -1;
>      struct ifreq ifr;
> +    VIR_AUTOCLOSE(fd);
>
>      if ((fd = virNetDevSetupControl(ifname, &ifr)) < 0)
>          return -1;
> @@ -604,7 +576,7 @@ int virNetDevSetName(const char* ifname, const char *newifname)
>          virReportSystemError(ERANGE,
>                               _("Network interface name '%s' is too long"),
>                               newifname);
> -        goto cleanup;
> +        return -1;
>      }
>  # else
>      ifr.ifr_data = (caddr_t)newifname;
> @@ -614,14 +586,10 @@ int virNetDevSetName(const char* ifname, const char *newifname)
>          virReportSystemError(errno,
>                               _("Unable to rename '%s' to '%s'"),
>                               ifname, newifname);
> -        goto cleanup;
> +        return -1;
>      }
>
> -    ret = 0;
> -
> - cleanup:
> -    VIR_FORCE_CLOSE(fd);
> -    return ret;
> +    return 0;
>  }
>  #else
>  int virNetDevSetName(const char* ifname, const char *newifname)
> @@ -638,10 +606,9 @@ int virNetDevSetName(const char* ifname, const char *newifname)
>  static int
>  virNetDevSetIFFlag(const char *ifname, int flag, bool val)
>  {
> -    int fd = -1;
> -    int ret = -1;
>      struct ifreq ifr;
>      int ifflags;
> +    VIR_AUTOCLOSE(fd);
>
>      if ((fd = virNetDevSetupControl(ifname, &ifr)) < 0)
>          return -1;
> @@ -650,7 +617,7 @@ virNetDevSetIFFlag(const char *ifname, int flag, bool val)
>          virReportSystemError(errno,
>                               _("Cannot get interface flags on '%s'"),
>                               ifname);
> -        goto cleanup;
> +        return -1;
>      }
>
>      if (val)
> @@ -664,15 +631,11 @@ virNetDevSetIFFlag(const char *ifname, int flag, bool val)
>              virReportSystemError(errno,
>                                   _("Cannot set interface flags on '%s'"),
>                                   ifname);
> -            goto cleanup;
> +            return -1;
>          }
>      }
>
> -    ret = 0;
> -
> - cleanup:
> -    VIR_FORCE_CLOSE(fd);
> -    return ret;
> +    return 0;
>  }
>  #else
>  static int
> @@ -765,9 +728,8 @@ virNetDevSetRcvAllMulti(const char *ifname,
>  static int
>  virNetDevGetIFFlag(const char *ifname, int flag, bool *val)
>  {
> -    int fd = -1;
> -    int ret = -1;
>      struct ifreq ifr;
> +    VIR_AUTOCLOSE(fd);
>
>      if ((fd = virNetDevSetupControl(ifname, &ifr)) < 0)
>          return -1;
> @@ -776,15 +738,11 @@ virNetDevGetIFFlag(const char *ifname, int flag, bool *val)
>          virReportSystemError(errno,
>                               _("Cannot get interface flags on '%s'"),
>                               ifname);
> -        goto cleanup;
> +        return -1;
>      }
>
>      *val = (ifr.ifr_flags & flag) ? true : false;
> -    ret = 0;
> -
> - cleanup:
> -    VIR_FORCE_CLOSE(fd);
> -    return ret;
> +    return 0;
>  }
>  #else
>  static int
> @@ -909,9 +867,9 @@ char *virNetDevGetName(int ifindex)
>  #if defined(SIOCGIFINDEX) && defined(HAVE_STRUCT_IFREQ)
>  int virNetDevGetIndex(const char *ifname, int *ifindex)
>  {
> -    int ret = -1;
>      struct ifreq ifreq;
> -    int fd = socket(VIR_NETDEV_FAMILY, SOCK_DGRAM, 0);
> +    VIR_AUTOCLOSE(fd);
> +    socket(VIR_NETDEV_FAMILY, SOCK_DGRAM, 0);

^this could not potentially work...

Erik




More information about the libvir-list mailing list