From f7e18208e19adaacd0d84b8a47c999318f0f6192 Mon Sep 17 00:00:00 2001 From: Laine Stump Date: Thu, 21 Jul 2011 22:48:32 -0400 Subject: [PATCH] util: make interface.c functions consistently return < 0 on error All of the functions in util/interface.c were returning 0 on success, but some returned -1 on error, and some returned a positive value (usually the value of errno, but sometimes just 1). Libvirt's standard is to return < 0 on error (in the case of functions that need to return errno, -errno is returned. This patch modifies all functions in interface.c to consistently return < 0 on error, and makes changes to callers of those functions where necessary. --- src/nwfilter/nwfilter_gentech_driver.c | 8 +- src/nwfilter/nwfilter_learnipaddr.c | 4 +- src/util/interface.c | 123 +++++++++++++------------ src/util/macvtap.c | 10 +- 4 files changed, 73 insertions(+), 72 deletions(-) diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c index bf44fc4ed9..7d9871a146 100644 --- a/src/nwfilter/nwfilter_gentech_driver.c +++ b/src/nwfilter/nwfilter_gentech_driver.c @@ -715,7 +715,7 @@ virNWFilterInstantiate(virConnectPtr conn, if (teardownOld && rc == 0) techdriver->tearOldRules(conn, ifname); - if (rc == 0 && ifaceCheck(false, ifname, NULL, ifindex)) { + if (rc == 0 && (ifaceCheck(false, ifname, NULL, ifindex) < 0)) { /* interface changed/disppeared */ techdriver->allTeardown(ifname); rc = 1; @@ -898,7 +898,7 @@ _virNWFilterInstantiateFilter(virConnectPtr conn, int ifindex; int rc; - if (ifaceGetIndex(true, net->ifname, &ifindex)) + if (ifaceGetIndex(true, net->ifname, &ifindex) < 0) return 1; virNWFilterLockFilterUpdates(); @@ -954,8 +954,8 @@ virNWFilterInstantiateFilterLate(virConnectPtr conn, &foundNewFilter); if (rc) { /* something went wrong... 'DOWN' the interface */ - if (ifaceCheck(false, ifname, NULL, ifindex) != 0 || - ifaceDown(ifname)) { + if ((ifaceCheck(false, ifname, NULL, ifindex) < 0) || + (ifaceDown(ifname) < 0)) { /* assuming interface disappeared... */ _virNWFilterTeardownFilter(ifname); } diff --git a/src/nwfilter/nwfilter_learnipaddr.c b/src/nwfilter/nwfilter_learnipaddr.c index cf818a2121..034eedbeef 100644 --- a/src/nwfilter/nwfilter_learnipaddr.c +++ b/src/nwfilter/nwfilter_learnipaddr.c @@ -431,7 +431,7 @@ learnIPAddressThread(void *arg) req->status = 0; /* anything change to the VM's interface -- check at least once */ - if (ifaceCheck(false, req->ifname, NULL, req->ifindex)) { + if (ifaceCheck(false, req->ifname, NULL, req->ifindex) < 0) { req->status = ENODEV; goto done; } @@ -501,7 +501,7 @@ learnIPAddressThread(void *arg) } /* check whether VM's dev is still there */ - if (ifaceCheck(false, req->ifname, NULL, req->ifindex)) { + if (ifaceCheck(false, req->ifname, NULL, req->ifindex) < 0) { req->status = ENODEV; showError = false; break; diff --git a/src/util/interface.c b/src/util/interface.c index 7b1a296888..72c7f3d134 100644 --- a/src/util/interface.c +++ b/src/util/interface.c @@ -59,10 +59,10 @@ getFlags(int fd, const char *ifname, struct ifreq *ifr) { if (virStrncpy(ifr->ifr_name, ifname, strlen(ifname), sizeof(ifr->ifr_name)) == NULL) - return ENODEV; + return -ENODEV; if (ioctl(fd, SIOCGIFFLAGS, ifr) < 0) - return errno; + return -errno; return 0; } @@ -74,7 +74,7 @@ getFlags(int fd, const char *ifname, struct ifreq *ifr) { * @ifname : name of the interface * @flags : pointer to short holding the flags on success * - * Get the flags of the interface. Returns 0 on success, error code on failure. + * Get the flags of the interface. Returns 0 on success, -errno on failure. */ int ifaceGetFlags(const char *ifname, short *flags) { @@ -83,7 +83,7 @@ ifaceGetFlags(const char *ifname, short *flags) { int fd = socket(PF_PACKET, SOCK_DGRAM, 0); if (fd < 0) - return errno; + return -errno; rc = getFlags(fd, ifname, &ifr); @@ -100,7 +100,7 @@ ifaceIsUp(const char *ifname, bool *up) { short flags = 0; int rc = ifaceGetFlags(ifname, &flags); - if (rc) + if (rc < 0) return rc; *up = ((flags & IFF_UP) == IFF_UP); @@ -112,11 +112,12 @@ ifaceIsUp(const char *ifname, bool *up) { /* Note: Showstopper on cygwin is only missing PF_PACKET */ int + ifaceGetFlags(const char *ifname ATTRIBUTE_UNUSED, short *flags ATTRIBUTE_UNUSED) { ifaceError(VIR_ERR_INTERNAL_ERROR, "%s", _("ifaceGetFlags is not supported on non-linux platforms")); - return ENOSYS; + return -ENOSYS; } int @@ -125,7 +126,7 @@ ifaceIsUp(const char *ifname ATTRIBUTE_UNUSED, ifaceError(VIR_ERR_INTERNAL_ERROR, "%s", _("ifaceIsUp is not supported on non-linux platforms")); - return ENOSYS; + return -ENOSYS; } #endif /* __linux__ */ @@ -141,7 +142,7 @@ ifaceIsUp(const char *ifname ATTRIBUTE_UNUSED, * flagmask = (~0 ^ flagclear) * newflags = (curflags & flagmask) | flagset; * - * Returns 0 on success, errno on failure. + * Returns 0 on success, -errno on failure. */ #ifdef __linux__ static int chgIfaceFlags(const char *ifname, short flagclear, short flagset) { @@ -152,10 +153,10 @@ static int chgIfaceFlags(const char *ifname, short flagclear, short flagset) { int fd = socket(PF_PACKET, SOCK_DGRAM, 0); if (fd < 0) - return errno; + return -errno; rc = getFlags(fd, ifname, &ifr); - if (rc != 0) + if (rc < 0) goto cleanup; flags = (ifr.ifr_flags & flagmask) | flagset; @@ -164,7 +165,7 @@ static int chgIfaceFlags(const char *ifname, short flagclear, short flagset) { ifr.ifr_flags = flags; if (ioctl(fd, SIOCSIFFLAGS, &ifr) < 0) - rc = errno; + rc = -errno; } cleanup: @@ -180,7 +181,7 @@ cleanup: * * Function to control if an interface is activated (up, 1) or not (down, 0) * - * Returns 0 in case of success or an errno code in case of failure. + * Returns 0 on success, -errno on failure. */ int ifaceCtrl(const char *name, bool up) @@ -195,7 +196,7 @@ ifaceCtrl(const char *name, bool up) int ifaceCtrl(const char *name ATTRIBUTE_UNUSED, bool up ATTRIBUTE_UNUSED) { - return ENOSYS; + return -ENOSYS; } #endif /* __linux__ */ @@ -212,10 +213,10 @@ ifaceCtrl(const char *name ATTRIBUTE_UNUSED, bool up ATTRIBUTE_UNUSED) * it must have the given MAC address and if an interface index is * passed, it must also match the interface index. * - * Returns 0 on success, an error code on failure. - * ENODEV : if interface with given name does not exist or its interface - * index is different than the one passed - * EINVAL : if interface name is invalid (too long) + * Returns 0 on success, -errno on failure. + * -ENODEV : if interface with given name does not exist or its interface + * index is different than the one passed + * -EINVAL : if interface name is invalid (too long) */ #ifdef __linux__ int @@ -230,7 +231,7 @@ ifaceCheck(bool reportError, const char *ifname, if (macaddr != NULL) { fd = socket(PF_PACKET, SOCK_DGRAM, 0); if (fd < 0) - return errno; + return -errno; memset(&ifr, 0, sizeof(ifr)); @@ -240,7 +241,7 @@ ifaceCheck(bool reportError, const char *ifname, ifaceError(VIR_ERR_INTERNAL_ERROR, _("invalid interface name %s"), ifname); - rc = EINVAL; + rc = -EINVAL; goto cleanup; } @@ -249,12 +250,12 @@ ifaceCheck(bool reportError, const char *ifname, ifaceError(VIR_ERR_INTERNAL_ERROR, _("coud not get MAC address of interface %s"), ifname); - rc = errno; + rc = -errno; goto cleanup; } if (memcmp(&ifr.ifr_hwaddr.sa_data, macaddr, VIR_MAC_BUFLEN) != 0) { - rc = ENODEV; + rc = -ENODEV; goto cleanup; } } @@ -262,7 +263,7 @@ ifaceCheck(bool reportError, const char *ifname, if (ifindex != -1) { rc = ifaceGetIndex(reportError, ifname, &idx); if (rc == 0 && idx != ifindex) - rc = ENODEV; + rc = -ENODEV; } cleanup: @@ -279,7 +280,7 @@ ifaceCheck(bool reportError ATTRIBUTE_UNUSED, const unsigned char *macaddr ATTRIBUTE_UNUSED, int ifindex ATTRIBUTE_UNUSED) { - return ENOSYS; + return -ENOSYS; } #endif /* __linux__ */ @@ -294,9 +295,9 @@ ifaceCheck(bool reportError ATTRIBUTE_UNUSED, * * Get the index of an interface given its name. * - * Returns 0 on success, an error code on failure. - * ENODEV : if interface with given name does not exist - * EINVAL : if interface name is invalid (too long) + * Returns 0 on success, -errno on failure. + * -ENODEV : if interface with given name does not exist + * -EINVAL : if interface name is invalid (too long) */ #ifdef __linux__ int @@ -307,7 +308,7 @@ ifaceGetIndex(bool reportError, const char *ifname, int *ifindex) int fd = socket(PF_PACKET, SOCK_DGRAM, 0); if (fd < 0) - return errno; + return -errno; memset(&ifreq, 0, sizeof(ifreq)); @@ -317,7 +318,7 @@ ifaceGetIndex(bool reportError, const char *ifname, int *ifindex) ifaceError(VIR_ERR_INTERNAL_ERROR, _("invalid interface name %s"), ifname); - rc = EINVAL; + rc = -EINVAL; goto cleanup; } @@ -328,7 +329,7 @@ ifaceGetIndex(bool reportError, const char *ifname, int *ifindex) ifaceError(VIR_ERR_INTERNAL_ERROR, _("interface %s does not exist"), ifname); - rc = ENODEV; + rc = -ENODEV; } cleanup: @@ -349,7 +350,7 @@ ifaceGetIndex(bool reportError, _("ifaceGetIndex is not supported on non-linux platforms")); } - return ENOSYS; + return -ENOSYS; } #endif /* __linux__ */ @@ -364,15 +365,15 @@ ifaceGetVlanID(const char *vlanifname, int *vlanid) { int fd = socket(PF_PACKET, SOCK_DGRAM, 0); if (fd < 0) - return errno; + return -errno; if (virStrcpyStatic(vlanargs.device1, vlanifname) == NULL) { - rc = EINVAL; + rc = -EINVAL; goto cleanup; } if (ioctl(fd, SIOCGIFVLAN, &vlanargs) != 0) { - rc = errno; + rc = -errno; goto cleanup; } @@ -393,7 +394,7 @@ ifaceGetVlanID(const char *vlanifname ATTRIBUTE_UNUSED, ifaceError(VIR_ERR_INTERNAL_ERROR, "%s", _("ifaceGetVlanID is not supported on non-linux platforms")); - return ENOSYS; + return -ENOSYS; } #endif /* __linux__ */ @@ -404,7 +405,7 @@ ifaceGetVlanID(const char *vlanifname ATTRIBUTE_UNUSED, * * This function gets the @macaddr for a given interface @ifname. * - * Returns 0 in case of success or an errno code in case of failure. + * Returns 0 on success, -errno on failure. */ #ifdef __linux__ int @@ -416,20 +417,20 @@ ifaceGetMacAddress(const char *ifname, int rc = 0; if (!ifname) - return EINVAL; + return -EINVAL; fd = socket(AF_INET, SOCK_STREAM, 0); if (fd < 0) - return errno; + return -errno; memset(&ifr, 0, sizeof(struct ifreq)); if (virStrcpyStatic(ifr.ifr_name, ifname) == NULL) { - rc = EINVAL; + rc = -EINVAL; goto cleanup; } if (ioctl(fd, SIOCGIFHWADDR, (char *)&ifr) != 0) { - rc = errno; + rc = -errno; goto cleanup; } @@ -446,7 +447,7 @@ int ifaceGetMacAddress(const char *ifname ATTRIBUTE_UNUSED, unsigned char *macaddr ATTRIBUTE_UNUSED) { - return ENOSYS; + return -ENOSYS; } #endif /* __linux__ */ @@ -459,7 +460,7 @@ ifaceGetMacAddress(const char *ifname ATTRIBUTE_UNUSED, * This function sets the @macaddr for a given interface @ifname. This * gets rid of the kernel's automatically assigned random MAC. * - * Returns 0 in case of success or an errno code in case of failure. + * Returns 0 on success, -errno on failure. */ #ifdef __linux__ int @@ -471,27 +472,27 @@ ifaceSetMacAddress(const char *ifname, int rc = 0; if (!ifname) - return EINVAL; + return -EINVAL; fd = socket(AF_INET, SOCK_STREAM, 0); if (fd < 0) - return errno; + return -errno; memset(&ifr, 0, sizeof(struct ifreq)); if (virStrcpyStatic(ifr.ifr_name, ifname) == NULL) { - rc = EINVAL; + rc = -EINVAL; goto cleanup; } /* To fill ifr.ifr_hdaddr.sa_family field */ if (ioctl(fd, SIOCGIFHWADDR, &ifr) != 0) { - rc = errno; + rc = -errno; goto cleanup; } memcpy(ifr.ifr_hwaddr.sa_data, macaddr, VIR_MAC_BUFLEN); - rc = ioctl(fd, SIOCSIFHWADDR, &ifr) == 0 ? 0 : errno; + rc = ioctl(fd, SIOCSIFHWADDR, &ifr) == 0 ? 0 : -errno; cleanup: VIR_FORCE_CLOSE(fd); @@ -504,7 +505,7 @@ int ifaceSetMacAddress(const char *ifname ATTRIBUTE_UNUSED, const unsigned char *macaddr ATTRIBUTE_UNUSED) { - return ENOSYS; + return -ENOSYS; } #endif /* __linux__ */ @@ -546,7 +547,7 @@ ifaceMacvtapLinkAdd(const char *type, struct nl_msg *nl_msg; struct nlattr *linkinfo, *info_data; - if (ifaceGetIndex(true, srcdev, &ifindex) != 0) + if (ifaceGetIndex(true, srcdev, &ifindex) < 0) return -1; *retry = 0; @@ -969,8 +970,8 @@ ifaceGetNthParent(int ifindex, const char *ifname, unsigned int nthParent, *nth = 0; - if (ifindex <= 0 && ifaceGetIndex(true, ifname, &ifindex) != 0) - return 1; + if (ifindex <= 0 && ifaceGetIndex(true, ifname, &ifindex) < 0) + return -1; while (!end && i <= nthParent) { rc = ifaceMacvtapLinkDump(true, ifname, ifindex, tb, &recvbuf, NULL); @@ -983,7 +984,7 @@ ifaceGetNthParent(int ifindex, const char *ifname, unsigned int nthParent, ifaceError(VIR_ERR_INTERNAL_ERROR, "%s", _("buffer for root interface name is too small")); VIR_FREE(recvbuf); - return 1; + return -1; } *parent_ifindex = ifindex; } @@ -1033,7 +1034,7 @@ ifaceGetNthParent(int ifindex ATTRIBUTE_UNUSED, * @linkdev: name of interface * @stateDir: directory to store old MAC address * - * Returns 0 on success, -1 in case of fatal error, error code otherwise. + * Returns 0 on success, -errno on failure. * */ int @@ -1046,7 +1047,7 @@ ifaceReplaceMacAddress(const unsigned char *macaddress, rc = ifaceGetMacAddress(linkdev, oldmac); - if (rc) { + if (rc < 0) { virReportSystemError(rc, _("Getting MAC address from '%s' " "to '%02x:%02x:%02x:%02x:%02x:%02x' failed."), @@ -1061,18 +1062,18 @@ ifaceReplaceMacAddress(const unsigned char *macaddress, stateDir, linkdev) < 0) { virReportOOMError(); - return errno; + return -errno; } virFormatMacAddr(oldmac, macstr); if (virFileWriteStr(path, macstr, O_CREAT|O_TRUNC|O_WRONLY) < 0) { virReportSystemError(errno, _("Unable to preserve mac for %s"), linkdev); - return errno; + return -errno; } } rc = ifaceSetMacAddress(linkdev, macaddress); - if (rc) { + if (rc < 0) { virReportSystemError(rc, _("Setting MAC address on '%s' to " "'%02x:%02x:%02x:%02x:%02x:%02x' failed."), @@ -1089,7 +1090,7 @@ ifaceReplaceMacAddress(const unsigned char *macaddress, * @linkdev: name of interface * @stateDir: directory containing old MAC address * - * Returns 0 on success, -1 in case of fatal error, error code otherwise. + * Returns 0 on success, -errno on failure. * */ int @@ -1106,11 +1107,11 @@ ifaceRestoreMacAddress(const char *linkdev, stateDir, linkdev) < 0) { virReportOOMError(); - return -1; + return -errno; } if (virFileReadAll(path, VIR_MAC_STRING_BUFLEN, &macstr) < 0) { - return errno; + return -errno; } if (virParseMacAddr(macstr, &oldmac[0]) != 0) { @@ -1118,12 +1119,12 @@ ifaceRestoreMacAddress(const char *linkdev, _("Cannot parse MAC address from '%s'"), oldmacname); VIR_FREE(macstr); - return -1; + return -EINVAL; } /*reset mac and remove file-ignore results*/ rc = ifaceSetMacAddress(linkdev, oldmac); - if (rc) { + if (rc < 0) { virReportSystemError(rc, _("Setting MAC address on '%s' to " "'%02x:%02x:%02x:%02x:%02x:%02x' failed."), diff --git a/src/util/macvtap.c b/src/util/macvtap.c index 7b97c6af04..86e52fa26c 100644 --- a/src/util/macvtap.c +++ b/src/util/macvtap.c @@ -309,14 +309,14 @@ openMacvtapTap(const char *tgifname, cr_ifname = tgifname; rc = ifaceMacvtapLinkAdd(type, macaddress, 6, tgifname, linkdev, macvtapMode, &do_retry); - if (rc) + if (rc < 0) return -1; } else { create_name: retries = 5; for (c = 0; c < 8192; c++) { snprintf(ifname, sizeof(ifname), MACVTAP_NAME_PATTERN, c); - if (ifaceGetIndex(false, ifname, &ifindex) == ENODEV) { + if (ifaceGetIndex(false, ifname, &ifindex) == -ENODEV) { rc = ifaceMacvtapLinkAdd(type, macaddress, 6, ifname, linkdev, macvtapMode, &do_retry); if (rc == 0) @@ -340,7 +340,7 @@ create_name: } rc = ifaceUp(cr_ifname); - if (rc != 0) { + if (rc < 0) { virReportSystemError(errno, _("cannot 'up' interface %s -- another " "macvtap device may be 'up' and have the same " @@ -838,7 +838,7 @@ getPhysdevAndVlan(const char *ifname, int *root_ifindex, char *root_ifname, if (nth == 0) break; if (*vlanid == -1) { - if (ifaceGetVlanID(root_ifname, vlanid)) + if (ifaceGetVlanID(root_ifname, vlanid) < 0) *vlanid = -1; } @@ -997,7 +997,7 @@ doPortProfileOp8021Qbh(const char *ifname, if (rc) goto err_exit; - if (ifaceGetIndex(true, physfndev, &ifindex) != 0) { + if (ifaceGetIndex(true, physfndev, &ifindex) < 0) { rc = 1; goto err_exit; }