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

Reply via email to