1:51 a.m.
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;
}
#else
--
1.7.3.4
6:31 a.m.
2011/7/6 Laine Stump <laine(a)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
8:10 a.m.
On 07/06/2011 07:31 AM, Matthias Bolte wrote:
2011/7/6 Laine Stump<laine(a)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(-)
> +
> +err_exit:
> + VIR_FORCE_CLOSE(fd);
> + return rc;
> }
HACKING suggests to call the label cleanup instead of err_exit in both
functions.
Yeah, but all the other functions in that file had err_exit, and I
wanted to be consistent within the file, but didn't want to rename
everything.
ACK, with the labels renamed.
Only if you'll ACK a separate patch that renames the other labels :-)
8:23 a.m.
2011/7/6 Laine Stump <laine(a)laine.org>:
On 07/06/2011 07:31 AM, Matthias Bolte wrote:
>
> 2011/7/6 Laine Stump<laine(a)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(-)
>> +
>> +err_exit:
>> + VIR_FORCE_CLOSE(fd);
>> + return rc;
>> }
>
> HACKING suggests to call the label cleanup instead of err_exit in both
> functions.
Yeah, but all the other functions in that file had err_exit, and I wanted to
be consistent within the file, but didn't want to rename everything.
> ACK, with the labels renamed.
Only if you'll ACK a separate patch that renames the other labels :-)
Oh, I didn't notice that. All err_exit in there qualify for being
renamed to cleanup, so there's no reason why I wouldn't ACK such a
patch :)
--
Matthias Bolte
http://photron.blogspot.com
8:38 a.m.
New subject: [libvirt] [PATCH] util: rename err_exit to cleanup in interface.c
This brings it in line with the recommendations in HACKING.
---
src/util/interface.c | 36 ++++++++++++++++++------------------
1 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/src/util/interface.c b/src/util/interface.c
index 32cd4eb..4037efb 100644
--- a/src/util/interface.c
+++ b/src/util/interface.c
@@ -156,7 +156,7 @@ static int chgIfaceFlags(const char *ifname, short flagclear, short
flagset) {
rc = getFlags(fd, ifname, &ifr);
if (rc != 0)
- goto err_exit;
+ goto cleanup;
flags = (ifr.ifr_flags & flagmask) | flagset;
@@ -167,7 +167,7 @@ static int chgIfaceFlags(const char *ifname, short flagclear, short
flagset) {
rc = errno;
}
-err_exit:
+cleanup:
VIR_FORCE_CLOSE(fd);
return rc;
}
@@ -241,7 +241,7 @@ ifaceCheck(bool reportError, const char *ifname,
_("invalid interface name %s"),
ifname);
rc = EINVAL;
- goto err_exit;
+ goto cleanup;
}
if (ioctl(fd, SIOCGIFHWADDR, &ifr) < 0) {
@@ -250,12 +250,12 @@ ifaceCheck(bool reportError, const char *ifname,
_("coud not get MAC address of interface %s"),
ifname);
rc = errno;
- goto err_exit;
+ goto cleanup;
}
if (memcmp(&ifr.ifr_hwaddr.sa_data, macaddr, VIR_MAC_BUFLEN) != 0) {
rc = ENODEV;
- goto err_exit;
+ goto cleanup;
}
}
@@ -265,7 +265,7 @@ ifaceCheck(bool reportError, const char *ifname,
rc = ENODEV;
}
- err_exit:
+ cleanup:
VIR_FORCE_CLOSE(fd);
return rc;
@@ -318,7 +318,7 @@ ifaceGetIndex(bool reportError, const char *ifname, int *ifindex)
_("invalid interface name %s"),
ifname);
rc = EINVAL;
- goto err_exit;
+ goto cleanup;
}
if (ioctl(fd, SIOCGIFINDEX, &ifreq) >= 0)
@@ -331,7 +331,7 @@ ifaceGetIndex(bool reportError, const char *ifname, int *ifindex)
rc = ENODEV;
}
-err_exit:
+cleanup:
VIR_FORCE_CLOSE(fd);
return rc;
@@ -368,17 +368,17 @@ ifaceGetVlanID(const char *vlanifname, int *vlanid) {
if (virStrcpyStatic(vlanargs.device1, vlanifname) == NULL) {
rc = EINVAL;
- goto err_exit;
+ goto cleanup;
}
if (ioctl(fd, SIOCGIFVLAN, &vlanargs) != 0) {
rc = errno;
- goto err_exit;
+ goto cleanup;
}
*vlanid = vlanargs.u.VID;
- err_exit:
+ cleanup:
VIR_FORCE_CLOSE(fd);
return rc;
@@ -636,7 +636,7 @@ ifaceMacvtapLinkAdd(const char *type,
if (nlComm(nl_msg, &recvbuf, &recvbuflen, 0) < 0) {
rc = -1;
- goto err_exit;
+ goto cleanup;
}
if (recvbuflen < NLMSG_LENGTH(0) || recvbuf == NULL)
@@ -675,7 +675,7 @@ ifaceMacvtapLinkAdd(const char *type,
goto malformed_resp;
}
-err_exit:
+cleanup:
nlmsg_free(nl_msg);
VIR_FREE(recvbuf);
@@ -760,7 +760,7 @@ ifaceLinkDel(const char *ifname)
if (nlComm(nl_msg, &recvbuf, &recvbuflen, 0) < 0) {
rc = -1;
- goto err_exit;
+ goto cleanup;
}
if (recvbuflen < NLMSG_LENGTH(0) || recvbuf == NULL)
@@ -789,7 +789,7 @@ ifaceLinkDel(const char *ifname)
goto malformed_resp;
}
-err_exit:
+cleanup:
nlmsg_free(nl_msg);
VIR_FREE(recvbuf);
@@ -892,13 +892,13 @@ ifaceMacvtapLinkDump(bool nltarget_kernel, const char *ifname, int
ifindex,
pid = getPidFunc();
if (pid == 0) {
rc = -1;
- goto err_exit;
+ goto cleanup;
}
}
if (nlComm(nl_msg, recvbuf, &recvbuflen, pid) < 0) {
rc = -1;
- goto err_exit;
+ goto cleanup;
}
if (recvbuflen < NLMSG_LENGTH(0) || *recvbuf == NULL)
@@ -935,7 +935,7 @@ ifaceMacvtapLinkDump(bool nltarget_kernel, const char *ifname, int
ifindex,
if (rc != 0)
VIR_FREE(*recvbuf);
-err_exit:
+cleanup:
nlmsg_free(nl_msg);
return rc;
--
1.7.3.4
8:50 a.m.
New subject: [libvirt] [PATCH] util: rename err_exit to cleanup in interface.c
2011/7/6 Laine Stump <laine(a)laine.org>:
This brings it in line with the recommendations in HACKING.
---
src/util/interface.c | 36 ++++++++++++++++++------------------
1 files changed, 18 insertions(+), 18 deletions(-)
ACK.
--
Matthias Bolte
http://photron.blogspot.com
4888
days inactive
4888
days old
5 comments
2 participants
participants (2)
-
Laine Stump -
Matthias Bolte