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@xxxxxxxxxxxxxx>
> ---
> 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
