[libvirt] [PATCH] util: close the ioctl socket at the end of if(Get|Set)MacAddress

[Matthias Bolte]

2011/7/6 Laine Stump <laine at laine.org>:
> Otherwise this will leak an fd each time one of these functions is
> called.
> ---
>  src/util/interface.c |   36 ++++++++++++++++++++++++++----------
>  1 files changed, 26 insertions(+), 10 deletions(-)
>
> diff --git a/src/util/interface.c b/src/util/interface.c
> index f486124..178a4dd 100644
> --- a/src/util/interface.c
> +++ b/src/util/interface.c
> @@ -413,6 +413,7 @@ ifaceGetMacAddress(const char *ifname,
>  {
>     struct ifreq ifr;
>     int fd;
> +    int rc = 0;
>
>     if (!ifname)
>         return EINVAL;
> @@ -422,15 +423,21 @@ ifaceGetMacAddress(const char *ifname,
>         return errno;
>
>     memset(&ifr, 0, sizeof(struct ifreq));
> -    if (virStrcpyStatic(ifr.ifr_name, ifname) == NULL)
> -        return EINVAL;
> +    if (virStrcpyStatic(ifr.ifr_name, ifname) == NULL) {
> +        rc = EINVAL;
> +        goto err_exit;
> +    }
>
> -    if (ioctl(fd, SIOCGIFHWADDR, (char *)&ifr) != 0)
> -        return errno;
> +    if (ioctl(fd, SIOCGIFHWADDR, (char *)&ifr) != 0) {
> +        rc = errno;
> +        goto err_exit;
> +    }
>
>     memcpy(macaddr, ifr.ifr_ifru.ifru_hwaddr.sa_data, VIR_MAC_BUFLEN);
>
> -    return 0;
> +err_exit:
> +    VIR_FORCE_CLOSE(fd);
> +    return rc;
>  }
>
>  #else
> @@ -461,6 +468,7 @@ ifaceSetMacAddress(const char *ifname,
>  {
>     struct ifreq ifr;
>     int fd;
> +    int rc = 0;
>
>     if (!ifname)
>         return EINVAL;
> @@ -470,16 +478,24 @@ ifaceSetMacAddress(const char *ifname,
>         return errno;
>
>     memset(&ifr, 0, sizeof(struct ifreq));
> -    if (virStrcpyStatic(ifr.ifr_name, ifname) == NULL)
> -        return EINVAL;
> +    if (virStrcpyStatic(ifr.ifr_name, ifname) == NULL) {
> +        rc = EINVAL;
> +        goto err_exit;
> +    }
>
>     /* To fill ifr.ifr_hdaddr.sa_family field */
> -    if (ioctl(fd, SIOCGIFHWADDR, &ifr) != 0)
> -        return errno;
> +    if (ioctl(fd, SIOCGIFHWADDR, &ifr) != 0) {
> +        rc = errno;
> +        goto err_exit;
> +    }
>
>     memcpy(ifr.ifr_hwaddr.sa_data, macaddr, VIR_MAC_BUFLEN);
>
> -    return ioctl(fd, SIOCSIFHWADDR, &ifr) == 0 ? 0 : errno;
> +    rc = ioctl(fd, SIOCSIFHWADDR, &ifr) == 0 ? 0 : errno;
> +
> +err_exit:
> +    VIR_FORCE_CLOSE(fd);
> +    return rc;
>  }

HACKING suggests to call the label cleanup instead of err_exit in both
functions.

ACK, with the labels renamed.

-- 
Matthias Bolte
http://photron.blogspot.com