[libvirt] [PATCH v3 14/35] util: file: use VIR_AUTOFREE instead of VIR_FREE for scalar types

By making use of GNU C's cleanup attribute handled by the VIR_AUTOFREE macro for declaring scalar variables, majority of the VIR_FREE calls can be dropped, which in turn leads to getting rid of most of our cleanup sections. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com&gt; --- src/util/virfile.c | 310 ++++++++++++++++++----------------------------------- 1 file changed, 102 insertions(+), 208 deletions(-) diff --git a/src/util/virfile.c b/src/util/virfile.c index 2690e2d..3a7445f 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -211,7 +211,7 @@ virFileWrapperFdNew(int *fd, const char *name, unsigned int flags) bool output = false; int pipefd[2] = { -1, -1 }; int mode = -1; - char *iohelper_path = NULL; + VIR_AUTOFREE(char *) iohelper_path = NULL; if (!flags) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -262,8 +262,6 @@ virFileWrapperFdNew(int *fd, const char *name, unsigned int flags) ret->cmd = virCommandNewArgList(iohelper_path, name, NULL); - VIR_FREE(iohelper_path); - if (output) { virCommandSetInputFD(ret->cmd, pipefd[0]); virCommandSetOutputFD(ret->cmd, fd); @@ -294,7 +292,6 @@ virFileWrapperFdNew(int *fd, const char *name, unsigned int flags) return ret; error: - VIR_FREE(iohelper_path); VIR_FORCE_CLOSE(pipefd[0]); VIR_FORCE_CLOSE(pipefd[1]); virFileWrapperFdFree(ret); @@ -492,7 +489,7 @@ virFileRewrite(const char *path, virFileRewriteFunc rewrite, const void *opaque) { - char *newfile = NULL; + VIR_AUTOFREE(char *) newfile = NULL; int fd = -1; int ret = -1; @@ -533,10 +530,8 @@ virFileRewrite(const char *path, cleanup: VIR_FORCE_CLOSE(fd); - if (newfile) { + if (newfile) unlink(newfile); - VIR_FREE(newfile); - } return ret; } @@ -763,7 +758,7 @@ int virFileLoopDeviceAssociate(const char *file, int lofd = -1; int fsfd = -1; struct loop_info64 lo; - char *loname = NULL; + VIR_AUTOFREE(char *) loname = NULL; int ret = -1; if ((lofd = virFileLoopDeviceOpen(&loname)) < 0) @@ -801,7 +796,6 @@ int virFileLoopDeviceAssociate(const char *file, ret = 0; cleanup: - VIR_FREE(loname); VIR_FORCE_CLOSE(fsfd); if (ret == -1) VIR_FORCE_CLOSE(lofd); @@ -816,8 +810,7 @@ int virFileLoopDeviceAssociate(const char *file, static int virFileNBDDeviceIsBusy(const char *dev_name) { - char *path; - int ret = -1; + VIR_AUTOFREE(char *) path = NULL; if (virAsprintf(&path, SYSFS_BLOCK_DIR "/%s/pid", dev_name) < 0) @@ -825,18 +818,14 @@ virFileNBDDeviceIsBusy(const char *dev_name) if (!virFileExists(path)) { if (errno == ENOENT) - ret = 0; + return 0; else virReportSystemError(errno, _("Cannot check NBD device %s pid"), dev_name); - goto cleanup; + return -1; } - ret = 1; - - cleanup: - VIR_FREE(path); - return ret; + return 1; } @@ -881,15 +870,13 @@ virFileNBDLoadDriver(void) "administratively prohibited")); return false; } else { - char *errbuf = NULL; + VIR_AUTOFREE(char *) errbuf = NULL; if ((errbuf = virKModLoad(NBD_DRIVER, true))) { - VIR_FREE(errbuf); virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Failed to load nbd module")); return false; } - VIR_FREE(errbuf); } return true; } @@ -899,8 +886,8 @@ int virFileNBDDeviceAssociate(const char *file, bool readonly, char **dev) { - char *nbddev = NULL; - char *qemunbd = NULL; + VIR_AUTOFREE(char *) nbddev = NULL; + VIR_AUTOFREE(char *) qemunbd = NULL; virCommandPtr cmd = NULL; int ret = -1; const char *fmtstr = NULL; @@ -948,8 +935,6 @@ int virFileNBDDeviceAssociate(const char *file, ret = 0; cleanup: - VIR_FREE(nbddev); - VIR_FREE(qemunbd); virCommandFree(cmd); return ret; } @@ -996,7 +981,6 @@ int virFileDeleteTree(const char *dir) { DIR *dh; struct dirent *de; - char *filepath = NULL; int ret = -1; int direrr; @@ -1008,6 +992,7 @@ int virFileDeleteTree(const char *dir) return -1; while ((direrr = virDirRead(dh, &de, dir)) > 0) { + VIR_AUTOFREE(char *) filepath = NULL; struct stat sb; if (virAsprintf(&filepath, "%s/%s", @@ -1031,8 +1016,6 @@ int virFileDeleteTree(const char *dir) goto cleanup; } } - - VIR_FREE(filepath); } if (direrr < 0) goto cleanup; @@ -1047,7 +1030,6 @@ int virFileDeleteTree(const char *dir) ret = 0; cleanup: - VIR_FREE(filepath); VIR_DIR_CLOSE(dh); return ret; } @@ -1205,7 +1187,7 @@ static int safezero_slow(int fd, off_t offset, off_t len) { int r; - char *buf; + VIR_AUTOFREE(char *) buf = NULL; unsigned long long remain, bytes; if (lseek(fd, offset, SEEK_SET) < 0) @@ -1226,15 +1208,12 @@ safezero_slow(int fd, off_t offset, off_t len) bytes = remain; r = safewrite(fd, buf, bytes); - if (r < 0) { - VIR_FREE(buf); + if (r < 0) return -1; - } /* safewrite() guarantees all data will be written */ remain -= bytes; } - VIR_FREE(buf); return 0; } @@ -1597,8 +1576,7 @@ virFileRelLinkPointsTo(const char *directory, const char *checkLink, const char *checkDest) { - char *candidate; - int ret; + VIR_AUTOFREE(char *) candidate = NULL; if (*checkLink == '/') return virFileLinkPointsTo(checkLink, checkDest); @@ -1610,9 +1588,7 @@ virFileRelLinkPointsTo(const char *directory, } if (virAsprintf(&candidate, "%s/%s", directory, checkLink) < 0) return -1; - ret = virFileLinkPointsTo(candidate, checkDest); - VIR_FREE(candidate); - return ret; + return virFileLinkPointsTo(candidate, checkDest); } @@ -1945,44 +1921,40 @@ virFileIsExecutable(const char *file) */ int virFileIsMountPoint(const char *file) { - char *parent = NULL; - int ret = -1; + VIR_AUTOFREE(char *) parent = NULL; + int ret; struct stat sb1, sb2; if (!(parent = mdir_name(file))) { virReportOOMError(); - goto cleanup; + return -1; } VIR_DEBUG("Comparing '%s' to '%s'", file, parent); if (stat(file, &sb1) < 0) { if (errno == ENOENT) - ret = 0; + return 0; else virReportSystemError(errno, _("Cannot stat '%s'"), file); - goto cleanup; + return -1; } if (stat(parent, &sb2) < 0) { virReportSystemError(errno, _("Cannot stat '%s'"), parent); - goto cleanup; + return -1; } - if (!S_ISDIR(sb1.st_mode)) { - ret = 0; - goto cleanup; - } + if (!S_ISDIR(sb1.st_mode)) + return 0; ret = sb1.st_dev != sb2.st_dev; VIR_DEBUG("Is mount %d", ret); - cleanup: - VIR_FREE(parent); return ret; } @@ -2165,7 +2137,7 @@ virFileAccessibleAs(const char *path, int mode, { pid_t pid = 0; int status, ret = 0; - gid_t *groups; + VIR_AUTOFREE(gid_t *) groups = NULL; int ngroups; if (uid == geteuid() && @@ -2178,13 +2150,10 @@ virFileAccessibleAs(const char *path, int mode, pid = virFork(); - if (pid < 0) { - VIR_FREE(groups); + if (pid < 0) return -1; - } if (pid) { /* parent */ - VIR_FREE(groups); if (virProcessWait(pid, &status, false) < 0) { /* virProcessWait() already reported error */ errno = EINTR; @@ -2280,7 +2249,7 @@ virFileOpenForked(const char *path, int openflags, mode_t mode, int recvfd_errno = 0; int fd = -1; int pair[2] = { -1, -1 }; - gid_t *groups; + VIR_AUTOFREE(gid_t *) groups = NULL; int ngroups; bool created = false; @@ -2298,16 +2267,12 @@ virFileOpenForked(const char *path, int openflags, mode_t mode, virReportSystemError(errno, _("failed to create socket needed for '%s'"), path); - VIR_FREE(groups); return ret; } pid = virFork(); - if (pid < 0) { - ret = -errno; - VIR_FREE(groups); - return ret; - } + if (pid < 0) + return -errno; if (pid == 0) { @@ -2372,7 +2337,6 @@ virFileOpenForked(const char *path, int openflags, mode_t mode, /* parent */ - VIR_FREE(groups); VIR_FORCE_CLOSE(pair[1]); do { @@ -2573,7 +2537,7 @@ virFileRemove(const char *path, { pid_t pid; int status = 0, ret = 0; - gid_t *groups; + VIR_AUTOFREE(gid_t *) groups = NULL; int ngroups; if (!virFileRemoveNeedsSetuid(path, uid, gid)) { @@ -2598,15 +2562,11 @@ virFileRemove(const char *path, pid = virFork(); - if (pid < 0) { - ret = -errno; - VIR_FREE(groups); - return ret; - } + if (pid < 0) + return -errno; if (pid) { /* parent */ /* wait for child to complete, and retrieve its exit code */ - VIR_FREE(groups); if (virProcessWait(pid, &status, 0) < 0) { /* virProcessWait() reports errno on waitpid failure, so we'll just @@ -2738,7 +2698,7 @@ virDirCreate(const char *path, struct stat st; pid_t pid; int status = 0, ret = 0; - gid_t *groups; + VIR_AUTOFREE(gid_t *) groups = NULL; int ngroups; bool created = false; @@ -2774,15 +2734,11 @@ virDirCreate(const char *path, pid = virFork(); - if (pid < 0) { - ret = -errno; - VIR_FREE(groups); - return ret; - } + if (pid < 0) + return -errno; if (pid) { /* parent */ /* wait for child to complete, and retrieve its exit code */ - VIR_FREE(groups); if (virProcessWait(pid, &status, 0) < 0) { /* virProcessWait() reports errno on waitpid failure, so we'll just @@ -3047,13 +3003,13 @@ int virFileChownFiles(const char *name, int ret = -1; int direrr; DIR *dir; - char *path = NULL; if (virDirOpen(&dir, name) < 0) return -1; while ((direrr = virDirRead(dir, &ent, name)) > 0) { - VIR_FREE(path); + VIR_AUTOFREE(char *) path = NULL; + if (virAsprintf(&path, "%s/%s", name, ent->d_name) < 0) goto cleanup; @@ -3075,8 +3031,6 @@ int virFileChownFiles(const char *name, ret = 0; cleanup: - VIR_FREE(path); - virDirClose(&dir); return ret; @@ -3138,19 +3092,14 @@ int virFileMakePathWithMode(const char *path, mode_t mode) { - int ret = -1; - char *tmp; + VIR_AUTOFREE(char *) tmp = NULL; if (VIR_STRDUP(tmp, path) < 0) { errno = ENOMEM; - goto cleanup; + return -1; } - ret = virFileMakePathHelper(tmp, mode); - - cleanup: - VIR_FREE(tmp); - return ret; + return virFileMakePathHelper(tmp, mode); } @@ -3158,8 +3107,7 @@ int virFileMakeParentPath(const char *path) { char *p; - char *tmp; - int ret = -1; + VIR_AUTOFREE(char *) tmp = NULL; VIR_DEBUG("path=%s", path); @@ -3170,15 +3118,11 @@ virFileMakeParentPath(const char *path) if ((p = strrchr(tmp, '/')) == NULL) { errno = EINVAL; - goto cleanup; + return -1; } *p = '\0'; - ret = virFileMakePathHelper(tmp, 0777); - - cleanup: - VIR_FREE(tmp); - return ret; + return virFileMakePathHelper(tmp, 0777); } @@ -3212,7 +3156,7 @@ virFileOpenTty(int *ttymaster, char **ttyName, int rawmode) * doesn't have to worry about that mess? */ int ret = -1; int slave = -1; - char *name = NULL; + VIR_AUTOFREE(char *) name = NULL; /* Unfortunately, we can't use the name argument of openpty, since * there is no guarantee on how large the buffer has to be. @@ -3273,7 +3217,6 @@ virFileOpenTty(int *ttymaster, char **ttyName, int rawmode) if (ret != 0) VIR_FORCE_CLOSE(*ttymaster); VIR_FORCE_CLOSE(slave); - VIR_FREE(name); return ret; } @@ -3373,21 +3316,17 @@ virFileSkipRoot(const char *path) int virFileAbsPath(const char *path, char **abspath) { - char *buf; - if (path[0] == '/') { if (VIR_STRDUP(*abspath, path) < 0) return -1; } else { - buf = getcwd(NULL, 0); + VIR_AUTOFREE(char *) buf = getcwd(NULL, 0); + if (buf == NULL) return -1; - if (virAsprintf(abspath, "%s/%s", buf, path) < 0) { - VIR_FREE(buf); + if (virAsprintf(abspath, "%s/%s", buf, path) < 0) return -1; - } - VIR_FREE(buf); } return 0; @@ -3487,7 +3426,7 @@ virFileRemoveLastComponent(char *path) int virFilePrintf(FILE *fp, const char *msg, ...) { va_list vargs; - char *str; + VIR_AUTOFREE(char *) str = NULL; int ret; va_start(vargs, msg); @@ -3501,8 +3440,6 @@ int virFilePrintf(FILE *fp, const char *msg, ...) ret = -1; } - VIR_FREE(str); - cleanup: va_end(vargs); @@ -3538,7 +3475,8 @@ int virFileIsSharedFSType(const char *path, int fstypes) { - char *dirpath, *p; + VIR_AUTOFREE(char *) dirpath = NULL; + char *p; struct statfs sb; int statfs_ret; @@ -3558,7 +3496,6 @@ virFileIsSharedFSType(const char *path, if ((p = strrchr(dirpath, '/')) == NULL) { virReportSystemError(EINVAL, _("Invalid relative path '%s'"), path); - VIR_FREE(dirpath); return -1; } @@ -3571,8 +3508,6 @@ virFileIsSharedFSType(const char *path, } while ((statfs_ret < 0) && (p != dirpath)); - VIR_FREE(dirpath); - if (statfs_ret < 0) { virReportSystemError(errno, _("cannot determine filesystem for '%s'"), @@ -3639,18 +3574,20 @@ virFileGetHugepageSize(const char *path, static int virFileGetDefaultHugepageSize(unsigned long long *size) { - int ret = -1; - char *meminfo, *c, *n, *unit; + VIR_AUTOFREE(char *) meminfo = NULL; + char *c; + char *n; + char *unit; if (virFileReadAll(PROC_MEMINFO, 4096, &meminfo) < 0) - goto cleanup; + return -1; if (!(c = strstr(meminfo, HUGEPAGESIZE_STR))) { virReportError(VIR_ERR_NO_SUPPORT, _("%s not found in %s"), HUGEPAGESIZE_STR, PROC_MEMINFO); - goto cleanup; + return -1; } c += strlen(HUGEPAGESIZE_STR); @@ -3663,13 +3600,10 @@ virFileGetDefaultHugepageSize(unsigned long long *size) virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to parse %s %s"), HUGEPAGESIZE_STR, c); - goto cleanup; + return -1; } - ret = 0; - cleanup: - VIR_FREE(meminfo); - return ret; + return 0; } # define PROC_MOUNTS "/proc/mounts" @@ -3963,10 +3897,8 @@ virFileCopyACLs(const char *src, int virFileComparePaths(const char *p1, const char *p2) { - int ret = -1; - char *res1, *res2; - - res1 = res2 = NULL; + VIR_AUTOFREE(char *) res1 = NULL; + VIR_AUTOFREE(char *) res2 = NULL; /* Assume p1 and p2 are symlinks, so try to resolve and canonicalize them. * Canonicalization fails for example on file systems names like 'proc' or @@ -3975,19 +3907,13 @@ virFileComparePaths(const char *p1, const char *p2) */ ignore_value(virFileResolveLink(p1, &res1)); if (!res1 && VIR_STRDUP(res1, p1) < 0) - goto cleanup; + return -1; ignore_value(virFileResolveLink(p2, &res2)); if (!res2 && VIR_STRDUP(res2, p2) < 0) - goto cleanup; - - ret = STREQ_NULLABLE(res1, res2); - - cleanup: - VIR_FREE(res1); - VIR_FREE(res2); + return -1; - return ret; + return STREQ_NULLABLE(res1, res2); } @@ -4131,25 +4057,22 @@ virFileInData(int fd ATTRIBUTE_UNUSED, int virFileReadValueInt(int *value, const char *format, ...) { - int ret = -1; - char *str = NULL; - char *path = NULL; + VIR_AUTOFREE(char *) str = NULL; + VIR_AUTOFREE(char *) path = NULL; va_list ap; va_start(ap, format); if (virVasprintf(&path, format, ap) < 0) { va_end(ap); - goto cleanup; + return -1; } va_end(ap); - if (!virFileExists(path)) { - ret = -2; - goto cleanup; - } + if (!virFileExists(path)) + return -2; if (virFileReadAll(path, INT_BUFSIZE_BOUND(*value), &str) < 0) - goto cleanup; + return -1; virStringTrimOptionalNewline(str); @@ -4157,14 +4080,10 @@ virFileReadValueInt(int *value, const char *format, ...) virReportError(VIR_ERR_INTERNAL_ERROR, _("Invalid integer value '%s' in file '%s'"), str, path); - goto cleanup; + return -1; } - ret = 0; - cleanup: - VIR_FREE(path); - VIR_FREE(str); - return ret; + return 0; } @@ -4181,25 +4100,22 @@ virFileReadValueInt(int *value, const char *format, ...) int virFileReadValueUint(unsigned int *value, const char *format, ...) { - int ret = -1; - char *str = NULL; - char *path = NULL; + VIR_AUTOFREE(char *) str = NULL; + VIR_AUTOFREE(char *) path = NULL; va_list ap; va_start(ap, format); if (virVasprintf(&path, format, ap) < 0) { va_end(ap); - goto cleanup; + return -1; } va_end(ap); - if (!virFileExists(path)) { - ret = -2; - goto cleanup; - } + if (!virFileExists(path)) + return -2; if (virFileReadAll(path, INT_BUFSIZE_BOUND(*value), &str) < 0) - goto cleanup; + return -1; virStringTrimOptionalNewline(str); @@ -4207,14 +4123,10 @@ virFileReadValueUint(unsigned int *value, const char *format, ...) virReportError(VIR_ERR_INTERNAL_ERROR, _("Invalid unsigned integer value '%s' in file '%s'"), str, path); - goto cleanup; + return -1; } - ret = 0; - cleanup: - VIR_FREE(path); - VIR_FREE(str); - return ret; + return 0; } @@ -4231,26 +4143,23 @@ virFileReadValueUint(unsigned int *value, const char *format, ...) int virFileReadValueScaledInt(unsigned long long *value, const char *format, ...) { - int ret = -1; - char *str = NULL; + VIR_AUTOFREE(char *) str = NULL; + VIR_AUTOFREE(char *) path = NULL; char *endp = NULL; - char *path = NULL; va_list ap; va_start(ap, format); if (virVasprintf(&path, format, ap) < 0) { va_end(ap); - goto cleanup; + return -1; } va_end(ap); - if (!virFileExists(path)) { - ret = -2; - goto cleanup; - } + if (!virFileExists(path)) + return -2; if (virFileReadAll(path, INT_BUFSIZE_BOUND(*value), &str) < 0) - goto cleanup; + return -1; virStringTrimOptionalNewline(str); @@ -4258,14 +4167,10 @@ virFileReadValueScaledInt(unsigned long long *value, const char *format, ...) virReportError(VIR_ERR_INTERNAL_ERROR, _("Invalid unsigned scaled integer value '%s' in file '%s'"), str, path); - goto cleanup; + return -1; } - ret = virScaleInteger(value, endp, 1024, ULLONG_MAX); - cleanup: - VIR_FREE(path); - VIR_FREE(str); - return ret; + return virScaleInteger(value, endp, 1024, ULLONG_MAX); } /* Arbitrarily sized number, feel free to change, but the function should be @@ -4285,37 +4190,30 @@ virFileReadValueScaledInt(unsigned long long *value, const char *format, ...) int virFileReadValueBitmap(virBitmapPtr *value, const char *format, ...) { - int ret = -1; - char *str = NULL; - char *path = NULL; + VIR_AUTOFREE(char *) str = NULL; + VIR_AUTOFREE(char *) path = NULL; va_list ap; va_start(ap, format); if (virVasprintf(&path, format, ap) < 0) { va_end(ap); - goto cleanup; + return -1; } va_end(ap); - if (!virFileExists(path)) { - ret = -2; - goto cleanup; - } + if (!virFileExists(path)) + return -2; if (virFileReadAll(path, VIR_FILE_READ_VALUE_STRING_MAX, &str) < 0) - goto cleanup; + return -1; virStringTrimOptionalNewline(str); *value = virBitmapParseUnlimited(str); if (!*value) - goto cleanup; + return -1; - ret = 0; - cleanup: - VIR_FREE(path); - VIR_FREE(str); - return ret; + return 0; } /** @@ -4333,30 +4231,26 @@ virFileReadValueBitmap(virBitmapPtr *value, const char *format, ...) int virFileReadValueString(char **value, const char *format, ...) { - int ret = -1; - char *str = NULL; - char *path = NULL; + int ret; + VIR_AUTOFREE(char *) str = NULL; + VIR_AUTOFREE(char *) path = NULL; va_list ap; va_start(ap, format); if (virVasprintf(&path, format, ap) < 0) { va_end(ap); - goto cleanup; + return -1; } va_end(ap); - if (!virFileExists(path)) { - ret = -2; - goto cleanup; - } + if (!virFileExists(path)) + return -2; ret = virFileReadAll(path, VIR_FILE_READ_VALUE_STRING_MAX, value); if (*value) virStringTrimOptionalNewline(*value); - cleanup: - VIR_FREE(path); - VIR_FREE(str); + return ret; } -- 1.8.3.1