The following pull request was submitted through Github. It can be accessed and reviewed at: https://github.com/lxc/lxc/pull/3291
This e-mail was sent by the LXC bot, direct replies will not reach the author unless they happen to be subscribed to this list. === Description (from pull-request) === Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com>
From 1dc51604cfffc83f152b8d3933e664143d468760 Mon Sep 17 00:00:00 2001 From: Christian Brauner <christian.brau...@ubuntu.com> Date: Wed, 11 Mar 2020 18:56:54 +0100 Subject: [PATCH 1/2] file_utils: cleanup macros and improvements Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com> --- src/lxc/file_utils.c | 126 +++++++++++++++++-------------------------- 1 file changed, 50 insertions(+), 76 deletions(-) diff --git a/src/lxc/file_utils.c b/src/lxc/file_utils.c index ad97057729..747e5c18cf 100644 --- a/src/lxc/file_utils.c +++ b/src/lxc/file_utils.c @@ -73,7 +73,7 @@ int lxc_write_openat(const char *dir, const char *filename, const void *buf, int lxc_write_to_file(const char *filename, const void *buf, size_t count, bool add_newline, mode_t mode) { - int fd, saved_errno; + __do_close_prot_errno int fd = -EBADF; ssize_t ret; fd = open(filename, O_WRONLY | O_TRUNC | O_CREAT | O_CLOEXEC, mode); @@ -82,30 +82,23 @@ int lxc_write_to_file(const char *filename, const void *buf, size_t count, ret = lxc_write_nointr(fd, buf, count); if (ret < 0) - goto out_error; + return -1; if ((size_t)ret != count) - goto out_error; + return -1; if (add_newline) { ret = lxc_write_nointr(fd, "\n", 1); if (ret != 1) - goto out_error; + return -1; } - close(fd); return 0; - -out_error: - saved_errno = errno; - close(fd); - errno = saved_errno; - return -1; } int lxc_read_from_file(const char *filename, void *buf, size_t count) { - int fd = -1, saved_errno; + __do_close_prot_errno int fd = -EBADF; ssize_t ret; fd = open(filename, O_RDONLY | O_CLOEXEC); @@ -126,19 +119,16 @@ int lxc_read_from_file(const char *filename, void *buf, size_t count) ret = lxc_read_nointr(fd, buf, count); } - saved_errno = errno; - close(fd); - errno = saved_errno; return ret; } ssize_t lxc_write_nointr(int fd, const void *buf, size_t count) { ssize_t ret; -again: - ret = write(fd, buf, count); - if (ret < 0 && errno == EINTR) - goto again; + + do { + ret = write(fd, buf, count); + } while (ret < 0 && errno == EINTR); return ret; } @@ -146,10 +136,10 @@ ssize_t lxc_write_nointr(int fd, const void *buf, size_t count) ssize_t lxc_send_nointr(int sockfd, void *buf, size_t len, int flags) { ssize_t ret; -again: - ret = send(sockfd, buf, len, flags); - if (ret < 0 && errno == EINTR) - goto again; + + do { + ret = send(sockfd, buf, len, flags); + } while (ret < 0 && errno == EINTR); return ret; } @@ -157,10 +147,10 @@ ssize_t lxc_send_nointr(int sockfd, void *buf, size_t len, int flags) ssize_t lxc_read_nointr(int fd, void *buf, size_t count) { ssize_t ret; -again: - ret = read(fd, buf, count); - if (ret < 0 && errno == EINTR) - goto again; + + do { + ret = read(fd, buf, count); + } while (ret < 0 && errno == EINTR); return ret; } @@ -168,10 +158,10 @@ ssize_t lxc_read_nointr(int fd, void *buf, size_t count) ssize_t lxc_recv_nointr(int sockfd, void *buf, size_t len, int flags) { ssize_t ret; -again: - ret = recv(sockfd, buf, len, flags); - if (ret < 0 && errno == EINTR) - goto again; + + do { + ret = recv(sockfd, buf, len, flags); + } while (ret < 0 && errno == EINTR); return ret; } @@ -180,21 +170,20 @@ ssize_t lxc_recvmsg_nointr_iov(int sockfd, struct iovec *iov, size_t iovlen, int flags) { ssize_t ret; - struct msghdr msg; + struct msghdr msg = { + .msg_iov = iov, + .msg_iovlen = iovlen, + }; - memset(&msg, 0, sizeof(msg)); - msg.msg_iov = iov; - msg.msg_iovlen = iovlen; - -again: - ret = recvmsg(sockfd, &msg, flags); - if (ret < 0 && errno == EINTR) - goto again; + do { + ret = recvmsg(sockfd, &msg, flags); + } while (ret < 0 && errno == EINTR); return ret; } -ssize_t lxc_read_nointr_expect(int fd, void *buf, size_t count, const void *expected_buf) +ssize_t lxc_read_nointr_expect(int fd, void *buf, size_t count, + const void *expected_buf) { ssize_t ret; @@ -205,15 +194,14 @@ ssize_t lxc_read_nointr_expect(int fd, void *buf, size_t count, const void *expe if ((size_t)ret != count) return -1; - if (expected_buf && memcmp(buf, expected_buf, count) != 0) { - errno = EINVAL; - return -1; - } + if (expected_buf && memcmp(buf, expected_buf, count) != 0) + return ret_set_errno(-1, EINVAL); return 0; } -ssize_t lxc_read_file_expect(const char *path, void *buf, size_t count, const void *expected_buf) +ssize_t lxc_read_file_expect(const char *path, void *buf, size_t count, + const void *expected_buf) { __do_close_prot_errno int fd = -EBADF; @@ -233,7 +221,7 @@ bool file_exists(const char *f) int print_to_file(const char *file, const char *content) { - FILE *f; + __do_fclose FILE *f = NULL; int ret = 0; f = fopen(file, "we"); @@ -243,14 +231,13 @@ int print_to_file(const char *file, const char *content) if (fprintf(f, "%s", content) != strlen(content)) ret = -1; - fclose(f); return ret; } int is_dir(const char *path) { - struct stat statbuf; int ret; + struct stat statbuf; ret = stat(path, &statbuf); if (ret == 0 && S_ISDIR(statbuf.st_mode)) @@ -264,8 +251,8 @@ int is_dir(const char *path) */ int lxc_count_file_lines(const char *fn) { - FILE *f; - char *line = NULL; + __do_free char *line = NULL; + __do_fclose FILE *f = NULL; size_t sz = 0; int n = 0; @@ -273,12 +260,9 @@ int lxc_count_file_lines(const char *fn) if (!f) return -1; - while (getline(&line, &sz, f) != -1) { + while (getline(&line, &sz, f) != -1) n++; - } - free(line); - fclose(f); return n; } @@ -338,11 +322,9 @@ bool fhas_fs_type(int fd, fs_type_magic magic_val) FILE *fopen_cloexec(const char *path, const char *mode) { - int open_mode = 0; - int step = 0; - int fd; - int saved_errno = 0; - FILE *ret; + __do_close_prot_errno int fd = -EBADF; + int open_mode = 0, step = 0; + FILE *f; if (!strncmp(mode, "r+", 2)) { open_mode = O_RDWR; @@ -366,32 +348,24 @@ FILE *fopen_cloexec(const char *path, const char *mode) for (; mode[step]; step++) if (mode[step] == 'x') open_mode |= O_EXCL; - open_mode |= O_CLOEXEC; - fd = open(path, open_mode, 0660); + fd = open(path, open_mode | O_CLOEXEC, 0660); if (fd < 0) return NULL; - ret = fdopen(fd, mode); - saved_errno = errno; - if (!ret) - close(fd); - errno = saved_errno; - return ret; + f = fdopen(fd, mode); + if (f) + move_fd(fd); + return f; } ssize_t lxc_sendfile_nointr(int out_fd, int in_fd, off_t *offset, size_t count) { ssize_t ret; -again: - ret = sendfile(out_fd, in_fd, offset, count); - if (ret < 0) { - if (errno == EINTR) - goto again; - - return -1; - } + do { + ret = sendfile(out_fd, in_fd, offset, count); + } while (ret < 0 && errno == EINTR); return ret; } From f12584558b4bcd738ffb622a8730def95effbd89 Mon Sep 17 00:00:00 2001 From: Christian Brauner <christian.brau...@ubuntu.com> Date: Wed, 11 Mar 2020 19:24:02 +0100 Subject: [PATCH 2/2] utils: cleanup Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com> --- src/lxc/utils.c | 189 +++++++++++++++++------------------------------- 1 file changed, 66 insertions(+), 123 deletions(-) diff --git a/src/lxc/utils.c b/src/lxc/utils.c index 8a2eacaf80..7d996e3677 100644 --- a/src/lxc/utils.c +++ b/src/lxc/utils.c @@ -63,21 +63,20 @@ extern bool btrfs_try_remove_subvol(const char *path); static int _recursive_rmdir(const char *dirname, dev_t pdev, const char *exclude, int level, bool onedev) { + __do_closedir DIR *dir = NULL; + int failed = 0; + bool hadexclude = false; + int ret; struct dirent *direntp; - DIR *dir; - int ret, failed = 0; char pathname[PATH_MAX]; - bool hadexclude = false; dir = opendir(dirname); - if (!dir) { - ERROR("Failed to open \"%s\"", dirname); - return -1; - } + if (!dir) + return log_error(-1, "Failed to open \"%s\"", dirname); while ((direntp = readdir(dir))) { - struct stat mystat; int rc; + struct stat mystat; if (!strcmp(direntp->d_name, ".") || !strcmp(direntp->d_name, "..")) @@ -86,14 +85,14 @@ static int _recursive_rmdir(const char *dirname, dev_t pdev, rc = snprintf(pathname, PATH_MAX, "%s/%s", dirname, direntp->d_name); if (rc < 0 || rc >= PATH_MAX) { ERROR("The name of path is too long"); - failed=1; + failed = 1; continue; } if (!level && exclude && !strcmp(direntp->d_name, exclude)) { ret = rmdir(pathname); if (ret < 0) { - switch(errno) { + switch (errno) { case ENOTEMPTY: INFO("Not deleting snapshot \"%s\"", pathname); hadexclude = true; @@ -121,48 +120,38 @@ static int _recursive_rmdir(const char *dirname, dev_t pdev, } if (onedev && mystat.st_dev != pdev) { - /* TODO should we be checking /proc/self/mountinfo for - * pathname and not doing this if found? */ if (btrfs_try_remove_subvol(pathname)) INFO("Removed btrfs subvolume at \"%s\"", pathname); continue; } if (S_ISDIR(mystat.st_mode)) { - if (_recursive_rmdir(pathname, pdev, exclude, level+1, onedev) < 0) - failed=1; + if (_recursive_rmdir(pathname, pdev, exclude, level + 1, onedev) < 0) + failed = 1; } else { if (unlink(pathname) < 0) { SYSERROR("Failed to delete \"%s\"", pathname); - failed=1; + failed = 1; } } } if (rmdir(dirname) < 0 && !btrfs_try_remove_subvol(dirname) && !hadexclude) { SYSERROR("Failed to delete \"%s\"", dirname); - failed=1; - } - - ret = closedir(dir); - if (ret) { - SYSERROR("Failed to close directory \"%s\"", dirname); - failed=1; + failed = 1; } return failed ? -1 : 0; } -/* In overlayfs, st_dev is unreliable. So on overlayfs we don't do the - * lxc_rmdir_onedev() +/* + * In overlayfs, st_dev is unreliable. So on overlayfs we don't do the + * lxc_rmdir_onedev(). */ -static bool is_native_overlayfs(const char *path) +static inline bool is_native_overlayfs(const char *path) { - if (has_fs_type(path, OVERLAY_SUPER_MAGIC) || - has_fs_type(path, OVERLAYFS_SUPER_MAGIC)) - return true; - - return false; + return has_fs_type(path, OVERLAY_SUPER_MAGIC) || + has_fs_type(path, OVERLAYFS_SUPER_MAGIC); } /* returns 0 on success, -1 if there were any failures */ @@ -178,8 +167,7 @@ extern int lxc_rmdir_onedev(const char *path, const char *exclude) if (errno == ENOENT) return 0; - SYSERROR("Failed to stat \"%s\"", path); - return -1; + return log_error_errno(-1, errno, "Failed to stat \"%s\"", path); } return _recursive_rmdir(path, mystat.st_dev, exclude, 0, onedev); @@ -210,25 +198,20 @@ int mkdir_p(const char *dir, mode_t mode) const char *orig = dir; do { + __do_free char *makeme = NULL; int ret; - char *makeme; dir = tmp + strspn(tmp, "/"); tmp = dir + strcspn(dir, "/"); - errno = ENOMEM; makeme = strndup(orig, dir - orig); if (!makeme) - return -1; + return ret_set_errno(-1, ENOMEM); ret = mkdir(makeme, mode); - if (ret < 0 && errno != EEXIST) { - SYSERROR("Failed to create directory \"%s\"", makeme); - free(makeme); - return -1; - } + if (ret < 0 && errno != EEXIST) + return log_error_errno(-1, errno, "Failed to create directory \"%s\"", makeme); - free(makeme); } while (tmp != dir); return 0; @@ -237,36 +220,31 @@ int mkdir_p(const char *dir, mode_t mode) char *get_rundir() { char *rundir; + size_t len; const char *homedir; struct stat sb; if (stat(RUNTIME_PATH, &sb) < 0) return NULL; - if (geteuid() == sb.st_uid || getegid() == sb.st_gid) { - rundir = strdup(RUNTIME_PATH); - return rundir; - } + if (geteuid() == sb.st_uid || getegid() == sb.st_gid) + return strdup(RUNTIME_PATH); rundir = getenv("XDG_RUNTIME_DIR"); - if (rundir) { - rundir = strdup(rundir); - return rundir; - } + if (rundir) + return strdup(rundir); INFO("XDG_RUNTIME_DIR isn't set in the environment"); homedir = getenv("HOME"); - if (!homedir) { - ERROR("HOME isn't set in the environment"); - return NULL; - } + if (!homedir) + return log_error(NULL, "HOME isn't set in the environment"); - rundir = malloc(sizeof(char) * (17 + strlen(homedir))); + len = strlen(homedir) + 17; + rundir = malloc(sizeof(char) * len); if (!rundir) return NULL; - sprintf(rundir, "%s/.cache/lxc/run/", homedir); - + snprintf(rundir, len, "%s/.cache/lxc/run/", homedir); return rundir; } @@ -328,16 +306,15 @@ int lxc_wait_for_pid_status(pid_t pid) #ifdef HAVE_OPENSSL #include <openssl/evp.h> -static int do_sha1_hash(const char *buf, int buflen, unsigned char *md_value, unsigned int *md_len) +static int do_sha1_hash(const char *buf, int buflen, unsigned char *md_value, + unsigned int *md_len) { EVP_MD_CTX *mdctx; const EVP_MD *md; md = EVP_get_digestbyname("sha1"); - if(!md) { - printf("Unknown message digest: sha1\n"); - return -1; - } + if (!md) + return log_error(-1, "Unknown message digest: sha1\n"); mdctx = EVP_MD_CTX_create(); EVP_DigestInit_ex(mdctx, md, NULL); @@ -350,60 +327,37 @@ static int do_sha1_hash(const char *buf, int buflen, unsigned char *md_value, un int sha1sum_file(char *fnam, unsigned char *digest, unsigned int *md_len) { - char *buf; + __do_free char *buf = NULL; + __do_fclose FILE *f = NULL; int ret; - FILE *f; long flen; if (!fnam) return -1; f = fopen_cloexec(fnam, "r"); - if (!f) { - SYSERROR("Failed to open template \"%s\"", fnam); - return -1; - } + if (!f) + return log_error_errno(-1, errno, "Failed to open template \"%s\"", fnam); - if (fseek(f, 0, SEEK_END) < 0) { - SYSERROR("Failed to seek to end of template"); - fclose(f); - return -1; - } + if (fseek(f, 0, SEEK_END) < 0) + return log_error_errno(-1, errno, "Failed to seek to end of template"); - if ((flen = ftell(f)) < 0) { - SYSERROR("Failed to tell size of template"); - fclose(f); - return -1; - } + flen = ftell(f); + if (flen < 0) + return log_error_errno(-1, errno, "Failed to tell size of template"); - if (fseek(f, 0, SEEK_SET) < 0) { - SYSERROR("Failed to seek to start of template"); - fclose(f); - return -1; - } + if (fseek(f, 0, SEEK_SET) < 0) + return log_error_errno(-1, errno, "Failed to seek to start of template"); - if ((buf = malloc(flen+1)) == NULL) { - SYSERROR("Out of memory"); - fclose(f); - return -1; - } + buf = malloc(flen + 1); + if (!buf) + return log_error_errno(-1, ENOMEM, "Out of memory"); - if (fread(buf, 1, flen, f) != flen) { - SYSERROR("Failed to read template"); - free(buf); - fclose(f); - return -1; - } - - if (fclose(f) < 0) { - SYSERROR("Failed to close template"); - free(buf); - return -1; - } + if (fread(buf, 1, flen, f) != flen) + return log_error_errno(-1, errno, "Failed to read template"); buf[flen] = '\0'; ret = do_sha1_hash(buf, flen, (void *)digest, md_len); - free(buf); return ret; } #endif @@ -556,10 +510,8 @@ uid_t get_ns_uid(uid_t orig) uid_t nsid, hostid, range; f = fopen("/proc/self/uid_map", "re"); - if (!f) { - SYSERROR("Failed to open uid_map"); - return 0; - } + if (!f) + return log_error_errno(0, errno, "Failed to open uid_map"); while (getline(&line, &sz, f) != -1) { if (sscanf(line, "%u %u %u", &nsid, &hostid, &range) != 3) @@ -580,10 +532,8 @@ gid_t get_ns_gid(gid_t orig) gid_t nsid, hostid, range; f = fopen("/proc/self/gid_map", "re"); - if (!f) { - SYSERROR("Failed to open gid_map"); - return 0; - } + if (!f) + return log_error_errno(0, errno, "Failed to open gid_map"); while (getline(&line, &sz, f) != -1) { if (sscanf(line, "%u %u %u", &nsid, &hostid, &range) != 3) @@ -697,17 +647,12 @@ bool switch_to_ns(pid_t pid, const char *ns) return false; fd = open(nspath, O_RDONLY | O_CLOEXEC); - if (fd < 0) { - SYSERROR("Failed to open \"%s\"", nspath); - return false; - } + if (fd < 0) + return log_error_errno(false, errno, "Failed to open \"%s\"", nspath); ret = setns(fd, 0); - if (ret) { - SYSERROR("Failed to set process %d to \"%s\" of %d.", pid, ns, - fd); - return false; - } + if (ret) + return log_error_errno(false, errno, "Failed to set process %d to \"%s\" of %d", pid, ns, fd); return true; } @@ -756,7 +701,8 @@ bool detect_ramfs_rootfs(void) char *on_path(const char *cmd, const char *rootfs) { - char *entry = NULL, *path = NULL; + __do_free char *path = NULL; + char *entry = NULL; char cmdpath[PATH_MAX]; int ret; @@ -768,7 +714,7 @@ char *on_path(const char *cmd, const char *rootfs) if (!path) return NULL; - lxc_iterate_parts (entry, path, ":") { + lxc_iterate_parts(entry, path, ":") { if (rootfs) ret = snprintf(cmdpath, PATH_MAX, "%s/%s/%s", rootfs, entry, cmd); @@ -777,13 +723,10 @@ char *on_path(const char *cmd, const char *rootfs) if (ret < 0 || ret >= PATH_MAX) continue; - if (access(cmdpath, X_OK) == 0) { - free(path); + if (access(cmdpath, X_OK) == 0) return strdup(cmdpath); - } } - free(path); return NULL; }
_______________________________________________ lxc-devel mailing list lxc-devel@lists.linuxcontainers.org http://lists.linuxcontainers.org/listinfo/lxc-devel