[libvirt] [PATCHv2 0/5] Introduce VIR_AUTOCLOSE macro for closing fd automatically
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
Signed-off-by: Shi Lei <shi_lei@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
Signed-off-by: Shi Lei <shi_lei@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
Signed-off-by: Shi Lei <shi_lei@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
Signed-off-by: Shi Lei <shi_lei@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
On 09/12/2018 11:46 AM, Shi Lei wrote:
Signed-off-by: Shi Lei <shi_lei@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
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@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
Signed-off-by: Shi Lei <shi_lei@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
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
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
participants (2)
-
Michal Privoznik -
Shi Lei