4:46 a.m.
v1 here:
https://www.redhat.com/archives/libvir-list/2018-September/msg00319.html
Diff from v1:
- Change VIR_AUTOCLOSE macro (comments from Michal)
- Remove others except for patches in util (comments from Erik)
- Change sc_require_attribute_cleanup_initialization for this macro
Shi Lei (5):
util: file: introduce VIR_AUTOCLOSE macro to close fd of the file
automatically
cfg.mk: Change syntax-check rule for VIR_AUTOCLOSE variable
initialization
util: file: use VIR_AUTOCLOSE instead of VIR_FORCE_CLOSE
util: netdevbridge: use VIR_AUTOCLOSE instead of VIR_FORCE_CLOSE
util: netdev: use VIR_AUTOCLOSE instead of VIR_FORCE_CLOSE
cfg.mk | 2 +-
src/util/virfile.c | 21 ++--
src/util/virfile.h | 18 ++-
src/util/virnetdev.c | 251 +++++++++++++------------------------
src/util/virnetdevbridge.c | 120 ++++++------------
5 files changed, 146 insertions(+), 266 deletions(-)
--
2.17.1
4:46 a.m.
New subject: [libvirt] [PATCHv2 1/5] util: file: introduce VIR_AUTOCLOSE macro to close fd of the file automatically
Signed-off-by: Shi Lei <shi_lei(a)massclouds.com>
---
src/util/virfile.h | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/src/util/virfile.h b/src/util/virfile.h
index b30a1d3..2bc3cf0 100644
--- a/src/util/virfile.h
+++ b/src/util/virfile.h
@@ -54,6 +54,11 @@ int virFileClose(int *fdptr, virFileCloseFlags flags)
int virFileFclose(FILE **file, bool preserve_errno) ATTRIBUTE_RETURN_CHECK;
FILE *virFileFdopen(int *fdptr, const char *mode) ATTRIBUTE_RETURN_CHECK;
+static inline void virForceCloseHelper(int *fd)
+{
+ ignore_value(virFileClose(fd, VIR_FILE_CLOSE_PRESERVE_ERRNO));
+}
+
/* For use on normal paths; caller must check return value,
and failure sets errno per close. */
# define VIR_CLOSE(FD) virFileClose(&(FD), 0)
@@ -64,8 +69,7 @@ FILE *virFileFdopen(int *fdptr, const char *mode)
ATTRIBUTE_RETURN_CHECK;
/* For use on cleanup paths; errno is unaffected by close,
and no return value to worry about. */
-# define VIR_FORCE_CLOSE(FD) \
- ignore_value(virFileClose(&(FD), VIR_FILE_CLOSE_PRESERVE_ERRNO))
+# define VIR_FORCE_CLOSE(FD) virForceCloseHelper(&(FD))
# define VIR_FORCE_FCLOSE(FILE) ignore_value(virFileFclose(&(FILE), true))
/* Similar VIR_FORCE_CLOSE() but ignores EBADF errors since they are expected
@@ -80,6 +84,16 @@ FILE *virFileFdopen(int *fdptr, const char *mode)
ATTRIBUTE_RETURN_CHECK;
VIR_FILE_CLOSE_PRESERVE_ERRNO | \
VIR_FILE_CLOSE_DONT_LOG))
+/**
+ * VIR_AUTOCLOSE:
+ *
+ * Macro to automatically force close the fd by calling virForceCloseHelper
+ * when the fd goes out of scope. It's used to eliminate VIR_FORCE_CLOSE
+ * in cleanup sections.
+ */
+# define VIR_AUTOCLOSE __attribute__((cleanup(virForceCloseHelper))) int
+
+
/* Opaque type for managing a wrapper around a fd. */
struct _virFileWrapperFd;
--
2.17.1
4:46 a.m.
New subject: [libvirt] [PATCHv2 2/5] cfg.mk: change syntax-check rule for VIR_AUTOCLOSE variable initialization
Signed-off-by: Shi Lei <shi_lei(a)massclouds.com>
---
cfg.mk | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/cfg.mk b/cfg.mk
index 609ae86..eddd110 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -1063,7 +1063,7 @@ sc_prohibit_backslash_alignment:
# Rule to ensure that varibales declared using a cleanup macro are
# always initialized.
sc_require_attribute_cleanup_initialization:
- @prohibit='VIR_AUTO(FREE|PTR)\(.+\) *[^=]+;' \
+ @prohibit='VIR_AUTO((FREE|PTR)\(.+\)|CLOSE) *[^=]+;' \
in_vc_files='\.[chx]$$' \
halt='variable declared with a cleanup macro must be initialized' \
$(_sc_search_regexp)
--
2.17.1
4:46 a.m.
New subject: [libvirt] [PATCHv2 3/5] util: file: use VIR_AUTOCLOSE instead of VIR_FORCE_CLOSE
Signed-off-by: Shi Lei <shi_lei(a)massclouds.com>
---
src/util/virfile.c | 21 +++++++--------------
1 file changed, 7 insertions(+), 14 deletions(-)
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 01ebdb6..2366c11 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -1969,29 +1969,22 @@ int
virFileIsCDROM(const char *path)
{
struct stat st;
- int fd;
- int ret = -1;
+ VIR_AUTOCLOSE fd = -1;
if ((fd = open(path, O_RDONLY | O_NONBLOCK)) < 0)
- goto cleanup;
+ return -1;
if (fstat(fd, &st) < 0)
- goto cleanup;
+ return -1;
- if (!S_ISBLK(st.st_mode)) {
- ret = 0;
- goto cleanup;
- }
+ if (!S_ISBLK(st.st_mode))
+ return 0;
/* Attempt to detect via a CDROM specific ioctl */
if (ioctl(fd, CDROM_DRIVE_STATUS, CDSL_CURRENT) >= 0)
- ret = 1;
- else
- ret = 0;
+ return 1;
- cleanup:
- VIR_FORCE_CLOSE(fd);
- return ret;
+ return 0;
}
#else
--
2.17.1
4:46 a.m.
New subject: [libvirt] [PATCHv2 4/5] util: netdevbridge: use VIR_AUTOCLOSE instead of VIR_FORCE_CLOSE
Signed-off-by: Shi Lei <shi_lei(a)massclouds.com>
---
src/util/virnetdevbridge.c | 120 ++++++++++++-------------------------
1 file changed, 37 insertions(+), 83 deletions(-)
diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c
index ed2db27..e058898 100644
--- a/src/util/virnetdevbridge.c
+++ b/src/util/virnetdevbridge.c
@@ -81,9 +81,8 @@ static int virNetDevBridgeCmd(const char *brname,
void *arg,
size_t argsize)
{
- int s;
- int ret = -1;
struct ifdrv ifd;
+ VIR_AUTOCLOSE s = -1;
memset(&ifd, 0, sizeof(ifd));
@@ -97,19 +96,14 @@ static int virNetDevBridgeCmd(const char *brname,
virReportSystemError(ERANGE,
_("Network interface name '%s' is too
long"),
brname);
- goto cleanup;
+ return -1;
}
ifd.ifd_cmd = op;
ifd.ifd_len = argsize;
ifd.ifd_data = arg;
- ret = ioctl(s, SIOCSDRVSPEC, &ifd);
-
- cleanup:
- VIR_FORCE_CLOSE(s);
-
- return ret;
+ return ioctl(s, SIOCSDRVSPEC, &ifd);
}
#endif
@@ -167,10 +161,9 @@ static int virNetDevBridgeGet(const char *brname,
const char *paramname, /* sysfs param name */
unsigned long *value) /* current value */
{
- int ret = -1;
- int fd = -1;
struct ifreq ifr;
VIR_AUTOFREE(char *) path = NULL;
+ VIR_AUTOCLOSE fd = -1;
if (virAsprintf(&path, SYSFS_NET_DIR "%s/bridge/%s", brname, paramname)
< 0)
return -1;
@@ -180,26 +173,26 @@ static int virNetDevBridgeGet(const char *brname,
if (virFileReadAll(path, INT_BUFSIZE_BOUND(unsigned long),
&valuestr) < 0)
- goto cleanup;
+ return -1;
if (virStrToLong_ul(valuestr, NULL, 10, value) < 0) {
virReportSystemError(EINVAL,
_("Unable to get bridge %s %s"),
brname, paramname);
- goto cleanup;
+ return -1;
}
} else {
struct __bridge_info info;
unsigned long args[] = { BRCTL_GET_BRIDGE_INFO, (unsigned long)&info, 0, 0
};
if ((fd = virNetDevSetupControl(brname, &ifr)) < 0)
- goto cleanup;
+ return -1;
ifr.ifr_data = (char*)&args;
if (ioctl(fd, SIOCDEVPRIVATE, ifr) < 0) {
virReportSystemError(errno,
_("Unable to get bridge %s %s"), brname,
paramname);
- goto cleanup;
+ return -1;
}
if (STREQ(paramname, "stp_state")) {
@@ -209,14 +202,11 @@ static int virNetDevBridgeGet(const char *brname,
} else {
virReportSystemError(EINVAL,
_("Unable to get bridge %s %s"), brname,
paramname);
- goto cleanup;
+ return -1;
}
}
- ret = 0;
- cleanup:
- VIR_FORCE_CLOSE(fd);
- return ret;
+ return 0;
}
#endif /* __linux__ */
@@ -391,8 +381,7 @@ virNetDevBridgePortSetUnicastFlood(const char *brname
ATTRIBUTE_UNUSED,
static int
virNetDevBridgeCreateWithIoctl(const char *brname)
{
- int fd = -1;
- int ret = -1;
+ VIR_AUTOCLOSE fd = -1;
if ((fd = virNetDevSetupControl(NULL, NULL)) < 0)
return -1;
@@ -400,14 +389,10 @@ virNetDevBridgeCreateWithIoctl(const char *brname)
if (ioctl(fd, SIOCBRADDBR, brname) < 0) {
virReportSystemError(errno,
_("Unable to create bridge %s"), brname);
- goto cleanup;
+ return -1;
}
- ret = 0;
-
- cleanup:
- VIR_FORCE_CLOSE(fd);
- return ret;
+ return 0;
}
#endif
@@ -448,9 +433,8 @@ virNetDevBridgeCreate(const char *brname)
int
virNetDevBridgeCreate(const char *brname)
{
- int s;
struct ifreq ifr;
- int ret = - 1;
+ VIR_AUTOCLOSE s = -1;
if ((s = virNetDevSetupControl("bridge", &ifr)) < 0)
return -1;
@@ -458,16 +442,13 @@ virNetDevBridgeCreate(const char *brname)
if (ioctl(s, SIOCIFCREATE2, &ifr) < 0) {
virReportSystemError(errno, "%s",
_("Unable to create bridge device"));
- goto cleanup;
+ return -1;
}
if (virNetDevSetName(ifr.ifr_name, brname) == -1)
- goto cleanup;
+ return -1;
- ret = 0;
- cleanup:
- VIR_FORCE_CLOSE(s);
- return ret;
+ return 0;
}
#else
int virNetDevBridgeCreate(const char *brname)
@@ -490,8 +471,7 @@ int virNetDevBridgeCreate(const char *brname)
static int
virNetDevBridgeDeleteWithIoctl(const char *brname)
{
- int fd = -1;
- int ret = -1;
+ VIR_AUTOCLOSE fd = -1;
ignore_value(virNetDevSetOnline(brname, false));
@@ -501,14 +481,10 @@ virNetDevBridgeDeleteWithIoctl(const char *brname)
if (ioctl(fd, SIOCBRDELBR, brname) < 0) {
virReportSystemError(errno,
_("Unable to delete bridge %s"), brname);
- goto cleanup;
+ return -1;
}
- ret = 0;
-
- cleanup:
- VIR_FORCE_CLOSE(fd);
- return ret;
+ return 0;
}
#endif
@@ -541,9 +517,8 @@ virNetDevBridgeDelete(const char *brname)
int
virNetDevBridgeDelete(const char *brname)
{
- int s;
struct ifreq ifr;
- int ret = -1;
+ VIR_AUTOCLOSE s = -1;
if ((s = virNetDevSetupControl(brname, &ifr)) < 0)
return -1;
@@ -552,13 +527,10 @@ virNetDevBridgeDelete(const char *brname)
virReportSystemError(errno,
_("Unable to remove bridge %s"),
brname);
- goto cleanup;
+ return -1;
}
- ret = 0;
- cleanup:
- VIR_FORCE_CLOSE(s);
- return ret;
+ return 0;
}
#else
int virNetDevBridgeDelete(const char *brname ATTRIBUTE_UNUSED)
@@ -582,9 +554,8 @@ int virNetDevBridgeDelete(const char *brname ATTRIBUTE_UNUSED)
int virNetDevBridgeAddPort(const char *brname,
const char *ifname)
{
- int fd = -1;
- int ret = -1;
struct ifreq ifr;
+ VIR_AUTOCLOSE fd = -1;
if ((fd = virNetDevSetupControl(brname, &ifr)) < 0)
return -1;
@@ -592,19 +563,16 @@ int virNetDevBridgeAddPort(const char *brname,
if (!(ifr.ifr_ifindex = if_nametoindex(ifname))) {
virReportSystemError(ENODEV,
_("Unable to get interface index for %s"),
ifname);
- goto cleanup;
+ return -1;
}
if (ioctl(fd, SIOCBRADDIF, &ifr) < 0) {
virReportSystemError(errno,
_("Unable to add bridge %s port %s"), brname,
ifname);
- goto cleanup;
+ return -1;
}
- ret = 0;
- cleanup:
- VIR_FORCE_CLOSE(fd);
- return ret;
+ return 0;
}
#elif defined(HAVE_BSD_BRIDGE_MGMT)
int virNetDevBridgeAddPort(const char *brname,
@@ -651,9 +619,8 @@ int virNetDevBridgeAddPort(const char *brname,
int virNetDevBridgeRemovePort(const char *brname,
const char *ifname)
{
- int fd = -1;
- int ret = -1;
struct ifreq ifr;
+ VIR_AUTOCLOSE fd = -1;
if ((fd = virNetDevSetupControl(brname, &ifr)) < 0)
return -1;
@@ -662,19 +629,16 @@ int virNetDevBridgeRemovePort(const char *brname,
virReportSystemError(ENODEV,
_("Unable to get interface index for %s"),
ifname);
- goto cleanup;
+ return -1;
}
if (ioctl(fd, SIOCBRDELIF, &ifr) < 0) {
virReportSystemError(errno,
_("Unable to remove bridge %s port %s"), brname,
ifname);
- goto cleanup;
+ return -1;
}
- ret = 0;
- cleanup:
- VIR_FORCE_CLOSE(fd);
- return ret;
+ return 0;
}
#elif defined(HAVE_BSD_BRIDGE_MGMT)
int virNetDevBridgeRemovePort(const char *brname,
@@ -723,19 +687,14 @@ int virNetDevBridgeRemovePort(const char *brname,
int virNetDevBridgeSetSTPDelay(const char *brname,
int delay)
{
- int fd = -1;
- int ret = -1;
struct ifreq ifr;
+ VIR_AUTOCLOSE fd = -1;
if ((fd = virNetDevSetupControl(brname, &ifr)) < 0)
- goto cleanup;
+ return -1;
- ret = virNetDevBridgeSet(brname, "forward_delay", MS_TO_JIFFIES(delay),
+ return virNetDevBridgeSet(brname, "forward_delay", MS_TO_JIFFIES(delay),
fd, &ifr);
-
- cleanup:
- VIR_FORCE_CLOSE(fd);
- return ret;
}
@@ -776,19 +735,14 @@ int virNetDevBridgeGetSTPDelay(const char *brname,
int virNetDevBridgeSetSTP(const char *brname,
bool enable)
{
- int fd = -1;
- int ret = -1;
struct ifreq ifr;
+ VIR_AUTOCLOSE fd = -1;
if ((fd = virNetDevSetupControl(brname, &ifr)) < 0)
- goto cleanup;
+ return -1;
- ret = virNetDevBridgeSet(brname, "stp_state", enable ? 1 : 0,
+ return virNetDevBridgeSet(brname, "stp_state", enable ? 1 : 0,
fd, &ifr);
-
- cleanup:
- VIR_FORCE_CLOSE(fd);
- return ret;
}
--
2.17.1
10:38 a.m.
New subject: [libvirt] [PATCHv2 4/5] util: netdevbridge: use VIR_AUTOCLOSE instead of VIR_FORCE_CLOSE
On 09/12/2018 11:46 AM, Shi Lei wrote:
Signed-off-by: Shi Lei <shi_lei(a)massclouds.com>
---
src/util/virnetdevbridge.c | 120 ++++++++++++-------------------------
1 file changed, 37 insertions(+), 83 deletions(-)
diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c
index ed2db27..e058898 100644
--- a/src/util/virnetdevbridge.c
+++ b/src/util/virnetdevbridge.c
@@ -723,19 +687,14 @@ int virNetDevBridgeRemovePort(const char
*brname,
int virNetDevBridgeSetSTPDelay(const char *brname,
int delay)
{
- int fd = -1;
- int ret = -1;
struct ifreq ifr;
+ VIR_AUTOCLOSE fd = -1;
if ((fd = virNetDevSetupControl(brname, &ifr)) < 0)
- goto cleanup;
+ return -1;
- ret = virNetDevBridgeSet(brname, "forward_delay", MS_TO_JIFFIES(delay),
+ return virNetDevBridgeSet(brname, "forward_delay", MS_TO_JIFFIES(delay),
fd, &ifr);
Misaligned arguments.
-
- cleanup:
- VIR_FORCE_CLOSE(fd);
- return ret;
}
@@ -776,19 +735,14 @@ int virNetDevBridgeGetSTPDelay(const char *brname,
int virNetDevBridgeSetSTP(const char *brname,
bool enable)
{
- int fd = -1;
- int ret = -1;
struct ifreq ifr;
+ VIR_AUTOCLOSE fd = -1;
if ((fd = virNetDevSetupControl(brname, &ifr)) < 0)
- goto cleanup;
+ return -1;
- ret = virNetDevBridgeSet(brname, "stp_state", enable ? 1 : 0,
+ return virNetDevBridgeSet(brname, "stp_state", enable ? 1 : 0,
fd, &ifr);
-
And again.
Michal
8:12 p.m.
New subject: [libvirt] [PATCHv2 4/5] util: netdevbridge: use VIR_AUTOCLOSE instead of VIR_FORCE_CLOSE
On 2018-09-12 at 23:38, Michal Privoznik wrote:
On 09/12/2018 11:46 AM, Shi Lei wrote:
> Signed-off-by: Shi Lei <shi_lei(a)massclouds.com>
> ---
> src/util/virnetdevbridge.c | 120 ++++++++++++-------------------------
> 1 file changed, 37 insertions(+), 83 deletions(-)
>
> diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c
> index ed2db27..e058898 100644
> --- a/src/util/virnetdevbridge.c
> +++ b/src/util/virnetdevbridge.c
> @@ -723,19 +687,14 @@ int virNetDevBridgeRemovePort(const char *brname,
> int virNetDevBridgeSetSTPDelay(const char *brname,
> int delay)
> {
> - int fd = -1;
> - int ret = -1;
> struct ifreq ifr;
> + VIR_AUTOCLOSE fd = -1;
>
> if ((fd = virNetDevSetupControl(brname, &ifr)) < 0)
> - goto cleanup;
> + return -1;
>
> - ret = virNetDevBridgeSet(brname, "forward_delay",
MS_TO_JIFFIES(delay),
> + return virNetDevBridgeSet(brname, "forward_delay",
MS_TO_JIFFIES(delay),
> fd, &ifr);
Misaligned arguments.
Sorry for it!
> -
> - cleanup:
> - VIR_FORCE_CLOSE(fd);
> - return ret;
> }
>
>
> @@ -776,19 +735,14 @@ int virNetDevBridgeGetSTPDelay(const char *brname,
> int virNetDevBridgeSetSTP(const char *brname,
> bool enable)
> {
> - int fd = -1;
> - int ret = -1;
> struct ifreq ifr;
> + VIR_AUTOCLOSE fd = -1;
>
> if ((fd = virNetDevSetupControl(brname, &ifr)) < 0)
> - goto cleanup;
> + return -1;
>
> - ret = virNetDevBridgeSet(brname, "stp_state", enable ? 1 : 0,
> + return virNetDevBridgeSet(brname, "stp_state", enable ? 1 : 0,
> fd, &ifr);
> -
And again.
Sorry. I would pay attention.
Michal
Thanks,
Shi Lei
4:46 a.m.
New subject: [libvirt] [PATCHv2 5/5] util: netdev: use VIR_AUTOCLOSE instead of VIR_FORCE_CLOSE
Signed-off-by: Shi Lei <shi_lei(a)massclouds.com>
---
src/util/virnetdev.c | 251 +++++++++++++++----------------------------
1 file changed, 85 insertions(+), 166 deletions(-)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 5d4ad24..a892251 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 = -1;
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 = -1;
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 = -1;
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 = -1;
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 = -1;
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 = -1;
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 = -1;
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 = -1;
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 = -1;
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,8 @@ 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);
if (fd < 0) {
virReportSystemError(errno, "%s",
@@ -925,13 +882,13 @@ int virNetDevGetIndex(const char *ifname, int *ifindex)
virReportSystemError(ERANGE,
_("invalid interface name %s"),
ifname);
- goto cleanup;
+ return -1;
}
if (ioctl(fd, SIOCGIFINDEX, &ifreq) < 0) {
virReportSystemError(errno,
_("Unable to get index for interface %s"),
ifname);
- goto cleanup;
+ return -1;
}
# ifdef HAVE_STRUCT_IFREQ_IFR_INDEX
@@ -939,11 +896,7 @@ int virNetDevGetIndex(const char *ifname, int *ifindex)
# else
*ifindex = ifreq.ifr_ifindex;
# endif
- ret = 0;
-
- cleanup:
- VIR_FORCE_CLOSE(fd);
- return ret;
+ return 0;
}
#else /* ! SIOCGIFINDEX */
int virNetDevGetIndex(const char *ifname ATTRIBUTE_UNUSED,
@@ -1013,8 +966,7 @@ int virNetDevGetVLanID(const char *ifname, int *vlanid)
struct vlan_ioctl_args vlanargs = {
.cmd = GET_VLAN_VID_CMD,
};
- int ret = -1;
- int fd = socket(PF_PACKET, SOCK_DGRAM, 0);
+ VIR_AUTOCLOSE fd = socket(PF_PACKET, SOCK_DGRAM, 0);
if (fd < 0) {
virReportSystemError(errno, "%s",
@@ -1026,22 +978,17 @@ int virNetDevGetVLanID(const char *ifname, int *vlanid)
virReportSystemError(ERANGE,
_("invalid interface name %s"),
ifname);
- goto cleanup;
+ return -1;
}
if (ioctl(fd, SIOCGIFVLAN, &vlanargs) != 0) {
virReportSystemError(errno,
_("Unable to get VLAN for interface %s"),
ifname);
- goto cleanup;
+ return -1;
}
*vlanid = vlanargs.u.VID;
- ret = 0;
-
- cleanup:
- VIR_FORCE_CLOSE(fd);
-
- return ret;
+ return 0;
}
#else /* ! SIOCGIFVLAN */
int virNetDevGetVLanID(const char *ifname ATTRIBUTE_UNUSED,
@@ -1070,55 +1017,43 @@ int virNetDevGetVLanID(const char *ifname ATTRIBUTE_UNUSED,
int virNetDevValidateConfig(const char *ifname,
const virMacAddr *macaddr, int ifindex)
{
- int fd = -1;
- int ret = -1;
struct ifreq ifr;
int idx;
int rc;
+ VIR_AUTOCLOSE fd = -1;
if ((rc = virNetDevExists(ifname)) < 0)
return -1;
- if (rc == 0) {
- ret = 0;
- goto cleanup;
- }
+ if (rc == 0)
+ return 0;
if (macaddr != NULL) {
if ((fd = virNetDevSetupControl(ifname, &ifr)) < 0)
return -1;
if (ioctl(fd, SIOCGIFHWADDR, &ifr) < 0) {
- if (errno == ENODEV) {
- ret = 0;
- goto cleanup;
- }
+ if (errno == ENODEV)
+ return 0;
+
virReportSystemError(errno,
_("could not get MAC address of interface
%s"),
ifname);
- goto cleanup;
+ return -1;
}
if (virMacAddrCmpRaw(macaddr,
- (unsigned char *)ifr.ifr_hwaddr.sa_data) != 0) {
- ret = 0;
- goto cleanup;
- }
+ (unsigned char *)ifr.ifr_hwaddr.sa_data) != 0)
+ return 0;
}
if (ifindex != -1) {
if (virNetDevGetIndex(ifname, &idx) < 0)
- goto cleanup;
- if (idx != ifindex) {
- ret = 0;
- goto cleanup;
- }
+ return -1;
+ if (idx != ifindex)
+ return 0;
}
- ret = 1;
-
- cleanup:
- VIR_FORCE_CLOSE(fd);
- return ret;
+ return 1;
}
#else
int virNetDevValidateConfig(const char *ifname ATTRIBUTE_UNUSED,
@@ -2649,9 +2584,8 @@ virNetDevGetLinkInfo(const char *ifname,
int virNetDevAddMulti(const char *ifname,
virMacAddrPtr macaddr)
{
- int fd = -1;
- int ret = -1;
struct ifreq ifr;
+ VIR_AUTOCLOSE fd = -1;
if ((fd = virNetDevSetupControl(ifname, &ifr)) < 0)
return -1;
@@ -2664,13 +2598,10 @@ int virNetDevAddMulti(const char *ifname,
virReportSystemError(errno,
_("Cannot add multicast MAC %s on '%s'
interface"),
virMacAddrFormat(macaddr, macstr), ifname);
- goto cleanup;
+ return -1;
}
- ret = 0;
- cleanup:
- VIR_FORCE_CLOSE(fd);
- return ret;
+ return 0;
}
#else
int virNetDevAddMulti(const char *ifname ATTRIBUTE_UNUSED,
@@ -2698,9 +2629,8 @@ int virNetDevAddMulti(const char *ifname ATTRIBUTE_UNUSED,
int virNetDevDelMulti(const char *ifname,
virMacAddrPtr macaddr)
{
- int fd = -1;
- int ret = -1;
struct ifreq ifr;
+ VIR_AUTOCLOSE fd = -1;
if ((fd = virNetDevSetupControl(ifname, &ifr)) < 0)
return -1;
@@ -2713,13 +2643,10 @@ int virNetDevDelMulti(const char *ifname,
virReportSystemError(errno,
_("Cannot add multicast MAC %s on '%s'
interface"),
virMacAddrFormat(macaddr, macstr), ifname);
- goto cleanup;
+ return -1;
}
- ret = 0;
- cleanup:
- VIR_FORCE_CLOSE(fd);
- return ret;
+ return 0;
}
#else
int virNetDevDelMulti(const char *ifname ATTRIBUTE_UNUSED,
@@ -3388,10 +3315,9 @@ int virNetDevSetCoalesce(const char *ifname,
virNetDevCoalescePtr coalesce,
bool update)
{
- int fd = -1;
- int ret = -1;
struct ifreq ifr;
struct ethtool_coalesce coal = {0};
+ VIR_AUTOCLOSE fd = -1;
if (!coalesce && !update)
return 0;
@@ -3433,7 +3359,7 @@ int virNetDevSetCoalesce(const char *ifname,
virReportSystemError(errno,
_("Cannot set coalesce info on '%s'"),
ifname);
- goto cleanup;
+ return -1;
}
if (coalesce) {
@@ -3467,10 +3393,7 @@ int virNetDevSetCoalesce(const char *ifname,
}
}
- ret = 0;
- cleanup:
- VIR_FORCE_CLOSE(fd);
- return ret;
+ return 0;
}
# else
int virNetDevSetCoalesce(const char *ifname,
@@ -3503,30 +3426,26 @@ virNetDevGetFeatures(const char *ifname,
virBitmapPtr *out)
{
struct ifreq ifr;
- int ret = -1;
- int fd = -1;
+ VIR_AUTOCLOSE fd = -1;
if (!(*out = virBitmapNew(VIR_NET_DEV_FEAT_LAST)))
return -1;
if ((fd = virNetDevSetupControl(ifname, &ifr)) < 0)
- goto cleanup;
+ return -1;
virNetDevGetEthtoolFeatures(*out, fd, &ifr);
if (virNetDevGetEthtoolGFeatures(*out, fd, &ifr) < 0)
- goto cleanup;
+ return -1;
if (virNetDevRDMAFeature(ifname, out) < 0)
- goto cleanup;
+ return -1;
if (virNetDevSwitchdevFeature(ifname, out) < 0)
- goto cleanup;
+ return -1;
- ret = 0;
- cleanup:
- VIR_FORCE_CLOSE(fd);
- return ret;
+ return 0;
}
#else
int
--
2.17.1
10:38 a.m.
On 09/12/2018 11:46 AM, Shi Lei wrote:
v1 here:
https://www.redhat.com/archives/libvir-list/2018-September/msg00319.html
Diff from v1:
- Change VIR_AUTOCLOSE macro (comments from Michal)
- Remove others except for patches in util (comments from Erik)
- Change sc_require_attribute_cleanup_initialization for this macro
Shi Lei (5):
util: file: introduce VIR_AUTOCLOSE macro to close fd of the file
automatically
cfg.mk: Change syntax-check rule for VIR_AUTOCLOSE variable
initialization
util: file: use VIR_AUTOCLOSE instead of VIR_FORCE_CLOSE
util: netdevbridge: use VIR_AUTOCLOSE instead of VIR_FORCE_CLOSE
util: netdev: use VIR_AUTOCLOSE instead of VIR_FORCE_CLOSE
cfg.mk | 2 +-
src/util/virfile.c | 21 ++--
src/util/virfile.h | 18 ++-
src/util/virnetdev.c | 251 +++++++++++++------------------------
src/util/virnetdevbridge.c | 120 ++++++------------
5 files changed, 146 insertions(+), 266 deletions(-)
ACKed and pushed.
Michal
8:13 p.m.
On 2018-09-12 at 23:38, Michal Privoznik wrote:
On 09/12/2018 11:46 AM, Shi Lei wrote:
> v1 here:
> https://www.redhat.com/archives/libvir-list/2018-September/msg00319.html
>
> Diff from v1:
> - Change VIR_AUTOCLOSE macro (comments from Michal)
> - Remove others except for patches in util (comments from Erik)
> - Change sc_require_attribute_cleanup_initialization for this macro
>
> Shi Lei (5):
> util: file: introduce VIR_AUTOCLOSE macro to close fd of the file
> automatically
> cfg.mk: Change syntax-check rule for VIR_AUTOCLOSE variable
> initialization
> util: file: use VIR_AUTOCLOSE instead of VIR_FORCE_CLOSE
> util: netdevbridge: use VIR_AUTOCLOSE instead of VIR_FORCE_CLOSE
> util: netdev: use VIR_AUTOCLOSE instead of VIR_FORCE_CLOSE
>
> cfg.mk | 2 +-
> src/util/virfile.c | 21 ++--
> src/util/virfile.h | 18 ++-
> src/util/virnetdev.c | 251 +++++++++++++------------------------
> src/util/virnetdevbridge.c | 120 ++++++------------
> 5 files changed, 146 insertions(+), 266 deletions(-)
>
ACKed and pushed.
Michal
Thanks,
Shi Lei
2262
days inactive
2263
days old
9 comments
2 participants
participants (2)
-
Michal Privoznik -
Shi Lei