[systemd-commits] 7 commits - src/login src/nspawn src/shared src/test

Zbigniew Jędrzejewski-Szmek zbyszek at kemper.freedesktop.org
Sun Sep 16 07:22:40 PDT 2012


 src/login/logind-seat.c    |    2 
 src/login/logind-session.c |    6 --
 src/nspawn/nspawn.c        |  117 +++++++++++++--------------------------------
 src/shared/install.c       |   26 +++-------
 src/shared/macro.h         |    1 
 src/shared/util.c          |    4 +
 src/shared/util.h          |    2 
 src/test/test-unit-file.c  |    2 
 8 files changed, 54 insertions(+), 106 deletions(-)

New commits:
commit 25ea79fe0756c3c46a16c0c19bf3380a30382a1c
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Sun Sep 16 16:14:11 2012 +0200

    nspawn: use automatic cleanup for umask

diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
index 0220ec4..b494045 100644
--- a/src/nspawn/nspawn.c
+++ b/src/nspawn/nspawn.c
@@ -435,7 +435,7 @@ static int copy_devnodes(const char *dest) {
 
         const char *d;
         int r = 0;
-        mode_t u;
+        mode_t _cleanup_umask_ u;
 
         assert(dest);
 
@@ -479,8 +479,6 @@ static int copy_devnodes(const char *dest) {
                 }
         }
 
-        umask(u);
-
         return r;
 }
 
@@ -488,7 +486,7 @@ static int setup_dev_console(const char *dest, const char *console) {
         struct stat st;
         char _cleanup_free_ *to = NULL;
         int r;
-        mode_t u;
+        mode_t _cleanup_umask_ u;
 
         assert(dest);
         assert(console);
@@ -497,25 +495,21 @@ static int setup_dev_console(const char *dest, const char *console) {
 
         if (stat(console, &st) < 0) {
                 log_error("Failed to stat %s: %m", console);
-                r = -errno;
-                goto finish;
+                return -errno;
 
         } else if (!S_ISCHR(st.st_mode)) {
-                log_error("/dev/console is not a char device.");
-                r = -EIO;
-                goto finish;
+                log_error("/dev/console is not a char device");
+                return -EIO;
         }
 
         r = chmod_and_chown(console, 0600, 0, 0);
         if (r < 0) {
                 log_error("Failed to correct access mode for TTY: %s", strerror(-r));
-                goto finish;
+                return r;
         }
 
-        if (asprintf(&to, "%s/dev/console", dest) < 0) {
-                r = log_oom();
-                goto finish;
-        }
+        if (asprintf(&to, "%s/dev/console", dest) < 0)
+                return log_oom();
 
         /* We need to bind mount the right tty to /dev/console since
          * ptys can only exist on pts file systems. To have something
@@ -526,26 +520,21 @@ static int setup_dev_console(const char *dest, const char *console) {
 
         if (mknod(to, (st.st_mode & ~07777) | 0600, st.st_rdev) < 0) {
                 log_error("mknod() for /dev/console failed: %m");
-                r = -errno;
-                goto finish;
+                return -errno;
         }
 
         if (mount(console, to, "bind", MS_BIND, NULL) < 0) {
                 log_error("Bind mount for /dev/console failed: %m");
-                r = -errno;
-                goto finish;
+                return -errno;
         }
 
-finish:
-        umask(u);
-
-        return r;
+        return 0;
 }
 
 static int setup_kmsg(const char *dest, int kmsg_socket) {
         char _cleanup_free_ *from = NULL, *to = NULL;
         int r, fd, k;
-        mode_t u;
+        mode_t _cleanup_umask_ u;
         union {
                 struct cmsghdr cmsghdr;
                 uint8_t buf[CMSG_SPACE(sizeof(int))];
@@ -565,39 +554,30 @@ static int setup_kmsg(const char *dest, int kmsg_socket) {
          * that writing blocks when nothing is reading. In order to
          * avoid any problems with containers deadlocking due to this
          * we simply make /dev/kmsg unavailable to the container. */
-        if (asprintf(&from, "%s/dev/kmsg", dest) < 0) {
-                r = log_oom();
-                goto finish;
-        }
-
-        if (asprintf(&to, "%s/proc/kmsg", dest) < 0) {
-                r = log_oom();
-                goto finish;
-        }
+        if (asprintf(&from, "%s/dev/kmsg", dest) < 0 ||
+            asprintf(&to, "%s/proc/kmsg", dest) < 0)
+                return log_oom();
 
         if (mkfifo(from, 0600) < 0) {
                 log_error("mkfifo() for /dev/kmsg failed: %m");
-                r = -errno;
-                goto finish;
+                return -errno;
         }
 
         r = chmod_and_chown(from, 0600, 0, 0);
         if (r < 0) {
                 log_error("Failed to correct access mode for /dev/kmsg: %s", strerror(-r));
-                goto finish;
+                return r;
         }
 
         if (mount(from, to, "bind", MS_BIND, NULL) < 0) {
                 log_error("Bind mount for /proc/kmsg failed: %m");
-                r = -errno;
-                goto finish;
+                return -errno;
         }
 
         fd = open(from, O_RDWR|O_NDELAY|O_CLOEXEC);
         if (fd < 0) {
                 log_error("Failed to open fifo: %m");
-                r = -errno;
-                goto finish;
+                return -errno;
         }
 
         zero(mh);
@@ -621,17 +601,12 @@ static int setup_kmsg(const char *dest, int kmsg_socket) {
 
         if (k < 0) {
                 log_error("Failed to send FIFO fd: %m");
-                r = -errno;
-                goto finish;
+                return -errno;
         }
 
         /* And now make the FIFO unavailable as /dev/kmsg... */
         unlink(from);
-
-finish:
-        umask(u);
-
-        return r;
+        return 0;
 }
 
 static int setup_hostname(void) {
diff --git a/src/shared/macro.h b/src/shared/macro.h
index f8c5656..c7ce7c8 100644
--- a/src/shared/macro.h
+++ b/src/shared/macro.h
@@ -191,5 +191,6 @@ static inline size_t IOVEC_INCREMENT(struct iovec *i, unsigned n, size_t k) {
 #define _cleanup_fclose_ __attribute__((cleanup(fclosep)))
 #define _cleanup_close_ __attribute__((cleanup(closep)))
 #define _cleanup_closedir_ __attribute__((cleanup(closedirp)))
+#define _cleanup_umask_ __attribute__((cleanup(umaskp)))
 
 #include "log.h"
diff --git a/src/shared/util.c b/src/shared/util.c
index add3fdc..27b6683 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -5827,3 +5827,7 @@ void closedirp(DIR **d) {
         if (*d)
                 closedir(*d);
 }
+
+void umaskp(mode_t *u) {
+        umask(*u);
+}
diff --git a/src/shared/util.h b/src/shared/util.h
index 2b75ba6..1a607cf 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -31,6 +31,7 @@
 #include <signal.h>
 #include <sched.h>
 #include <limits.h>
+#include <sys/types.h>
 #include <sys/stat.h>
 #include <dirent.h>
 #include <sys/resource.h>
@@ -533,3 +534,4 @@ void freep(void *p);
 void fclosep(FILE **f);
 void closep(int *fd);
 void closedirp(DIR **d);
+void umaskp(mode_t *u);

commit ed8b7a3ee55b27a06a54d2dfa39eec5e555e005b
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Sun Sep 16 15:21:48 2012 +0200

    nspawn: _cleanup_free_ more

diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
index c46f63b..0220ec4 100644
--- a/src/nspawn/nspawn.c
+++ b/src/nspawn/nspawn.c
@@ -380,7 +380,7 @@ static int setup_resolv_conf(const char *dest) {
 }
 
 static int setup_boot_id(const char *dest) {
-        char *from = NULL, *to = NULL;
+        char _cleanup_free_ *from = NULL, *to = NULL;
         sd_id128_t rnd;
         char as_uuid[37];
         int r;
@@ -391,21 +391,14 @@ static int setup_boot_id(const char *dest) {
          * the container gets a new one */
 
         from = strappend(dest, "/dev/proc-sys-kernel-random-boot-id");
-        if (!from) {
-                r = log_oom();
-                goto finish;
-        }
-
         to = strappend(dest, "/proc/sys/kernel/random/boot_id");
-        if (!to) {
-                r = log_oom();
-                goto finish;
-        }
+        if (!from || !to)
+                return log_oom();
 
         r = sd_id128_randomize(&rnd);
         if (r < 0) {
                 log_error("Failed to generate random boot id: %s", strerror(-r));
-                goto finish;
+                return r;
         }
 
         snprintf(as_uuid, sizeof(as_uuid),
@@ -416,7 +409,7 @@ static int setup_boot_id(const char *dest) {
         r = write_one_line_file(from, as_uuid);
         if (r < 0) {
                 log_error("Failed to write boot id: %s", strerror(-r));
-                goto finish;
+                return r;
         }
 
         if (mount(from, to, "bind", MS_BIND, NULL) < 0) {
@@ -426,11 +419,6 @@ static int setup_boot_id(const char *dest) {
                 mount(from, to, "bind", MS_BIND|MS_REMOUNT|MS_RDONLY, NULL);
 
         unlink(from);
-
-finish:
-        free(from);
-        free(to);
-
         return r;
 }
 
@@ -455,18 +443,13 @@ static int copy_devnodes(const char *dest) {
 
         NULSTR_FOREACH(d, devnodes) {
                 struct stat st;
-                char *from = NULL, *to = NULL;
+                char _cleanup_free_ *from = NULL, *to = NULL;
 
                 asprintf(&from, "/dev/%s", d);
                 asprintf(&to, "%s/dev/%s", dest, d);
 
                 if (!from || !to) {
-                        log_error("Failed to allocate devnode path");
-
-                        free(from);
-                        free(to);
-
-                        from = to = NULL;
+                        log_oom();
 
                         if (r == 0)
                                 r = -ENOMEM;
@@ -484,7 +467,7 @@ static int copy_devnodes(const char *dest) {
 
                 } else if (!S_ISCHR(st.st_mode) && !S_ISBLK(st.st_mode)) {
 
-                        log_error("%s is not a char or block device, cannot copy.", from);
+                        log_error("%s is not a char or block device, cannot copy", from);
                         if (r == 0)
                                 r = -EIO;
 
@@ -494,9 +477,6 @@ static int copy_devnodes(const char *dest) {
                         if (r == 0)
                                 r = -errno;
                 }
-
-                free(from);
-                free(to);
         }
 
         umask(u);
@@ -506,7 +486,7 @@ static int copy_devnodes(const char *dest) {
 
 static int setup_dev_console(const char *dest, const char *console) {
         struct stat st;
-        char *to = NULL;
+        char _cleanup_free_ *to = NULL;
         int r;
         mode_t u;
 
@@ -557,14 +537,13 @@ static int setup_dev_console(const char *dest, const char *console) {
         }
 
 finish:
-        free(to);
         umask(u);
 
         return r;
 }
 
 static int setup_kmsg(const char *dest, int kmsg_socket) {
-        char *from = NULL, *to = NULL;
+        char _cleanup_free_ *from = NULL, *to = NULL;
         int r, fd, k;
         mode_t u;
         union {
@@ -650,8 +629,6 @@ static int setup_kmsg(const char *dest, int kmsg_socket) {
         unlink(from);
 
 finish:
-        free(from);
-        free(to);
         umask(u);
 
         return r;

commit 6b2d0e85dce8762d6d65a97c7c7ae5e81806fe3e
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Sun Sep 16 15:09:47 2012 +0200

    nspawn: use automatic cleanup
    
    This one actually clears up a (totally harmless) memleak.

diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
index 770019b..c46f63b 100644
--- a/src/nspawn/nspawn.c
+++ b/src/nspawn/nspawn.c
@@ -46,6 +46,7 @@
 #include "log.h"
 #include "util.h"
 #include "mkdir.h"
+#include "macro.h"
 #include "audit.h"
 #include "missing.h"
 #include "cgroup-util.h"
@@ -283,7 +284,7 @@ static int mount_all(const char *dest) {
 
         unsigned k;
         int r = 0;
-        char *where;
+        char _cleanup_free_ *where = NULL;
 
         for (k = 0; k < ELEMENTSOF(mount_table); k++) {
                 int t;
@@ -300,7 +301,6 @@ static int mount_all(const char *dest) {
                 t = path_is_mount_point(where, true);
                 if (t < 0) {
                         log_error("Failed to detect whether %s is a mount point: %s", where, strerror(-t));
-                        free(where);
 
                         if (r == 0)
                                 r = t;
@@ -326,8 +326,6 @@ static int mount_all(const char *dest) {
                         if (r == 0)
                                 r = -errno;
                 }
-
-                free(where);
         }
 
         return r;

commit ede89845a49c735fa9e8a04bd80f6e94ea84aeb9
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Sun Sep 16 14:58:51 2012 +0200

    nspawn: mount tmpfs on /dev/shm
    
    Most things seem to function fine without /dev/shm, but it is expected
    to be there (quoting linux/Documentation/filesystems/tmpfs.txt:
    glibc 2.2 and above expects tmpfs to be mounted at /dev/shm for POSIX
    shared memory (shm_open, shm_unlink)).
    
    Since /tmp/ is already mounted as tmpfs, it would be enough to mkdir
    /tmp/shm and chmod it. Mounting it separately has the advantage that
    it can be easily remounted to change the quota.

diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
index 7f084ef..770019b 100644
--- a/src/nspawn/nspawn.c
+++ b/src/nspawn/nspawn.c
@@ -273,6 +273,7 @@ static int mount_all(const char *dest) {
                 { "sysfs",     "/sys",      "sysfs", NULL,       MS_RDONLY|MS_NOSUID|MS_NOEXEC|MS_NODEV, true  },
                 { "tmpfs",     "/dev",      "tmpfs", "mode=755", MS_NOSUID|MS_STRICTATIME,     true  },
                 { "/dev/pts",  "/dev/pts",  NULL,    NULL,       MS_BIND,                      true  },
+                { "tmpfs",     "/dev/shm",  "tmpfs", "mode=1777", MS_NOSUID|MS_NODEV|MS_STRICTATIME, true  },
                 { "tmpfs",     "/run",      "tmpfs", "mode=755", MS_NOSUID|MS_NODEV|MS_STRICTATIME, true  },
 #ifdef HAVE_SELINUX
                 { "/sys/fs/selinux", "/sys/fs/selinux", NULL, NULL, MS_BIND,                      false },  /* Bind mount first */

commit d8831ed55442cfe0a3ca54644282a7c27d26f1b0
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Sun Sep 16 12:55:28 2012 +0200

    install: use automatic cleanup

diff --git a/src/shared/install.c b/src/shared/install.c
index 0d38bcc..c6215fb 100644
--- a/src/shared/install.c
+++ b/src/shared/install.c
@@ -541,7 +541,7 @@ static int find_symlinks_in_scope(
                 UnitFileState *state) {
 
         int r;
-        char *path;
+        char _cleanup_free_ *path = NULL;
         bool same_name_link_runtime = false, same_name_link = false;
 
         assert(scope >= 0);
@@ -556,8 +556,6 @@ static int find_symlinks_in_scope(
                         return r;
 
                 r = find_symlinks(name, path, &same_name_link_runtime);
-                free(path);
-
                 if (r < 0)
                         return r;
                 else if (r > 0) {
@@ -572,8 +570,6 @@ static int find_symlinks_in_scope(
                 return r;
 
         r = find_symlinks(name, path, &same_name_link);
-        free(path);
-
         if (r < 0)
                 return r;
         else if (r > 0) {
@@ -603,7 +599,8 @@ int unit_file_mask(
                 UnitFileChange **changes,
                 unsigned *n_changes) {
 
-        char **i, *prefix;
+        char **i;
+        char _cleanup_free_ *prefix;
         int r;
 
         assert(scope >= 0);
@@ -614,7 +611,7 @@ int unit_file_mask(
                 return r;
 
         STRV_FOREACH(i, files) {
-                char *path;
+                char _cleanup_free_ *path = NULL;
 
                 if (!unit_name_is_valid(*i, true)) {
                         if (r == 0)
@@ -631,16 +628,13 @@ int unit_file_mask(
                 if (symlink("/dev/null", path) >= 0) {
                         add_file_change(changes, n_changes, UNIT_FILE_SYMLINK, path, "/dev/null");
 
-                        free(path);
                         continue;
                 }
 
                 if (errno == EEXIST) {
 
-                        if (null_or_empty_path(path) > 0) {
-                                free(path);
+                        if (null_or_empty_path(path) > 0)
                                 continue;
-                        }
 
                         if (force) {
                                 unlink(path);
@@ -650,7 +644,6 @@ int unit_file_mask(
                                         add_file_change(changes, n_changes, UNIT_FILE_UNLINK, path, NULL);
                                         add_file_change(changes, n_changes, UNIT_FILE_SYMLINK, path, "/dev/null");
 
-                                        free(path);
                                         continue;
                                 }
                         }
@@ -661,12 +654,8 @@ int unit_file_mask(
                         if (r == 0)
                                 r = -errno;
                 }
-
-                free(path);
         }
 
-        free(prefix);
-
         return r;
 }
 

commit d5891fdacf761130c9babdb5e50367504d29970c
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Sun Sep 16 12:35:46 2012 +0200

    install: treat non-existent directory as empty
    
    When looking for symlinks, it doesn't make sense to error-out if
    the directory is missing. The user might delete an empty directory.
    
    This check caused test-unit-file to fail when run before installation.

diff --git a/src/shared/install.c b/src/shared/install.c
index 1a69337..0d38bcc 100644
--- a/src/shared/install.c
+++ b/src/shared/install.c
@@ -524,8 +524,11 @@ static int find_symlinks(
         assert(same_name_link);
 
         fd = open(config_path, O_RDONLY|O_NONBLOCK|O_DIRECTORY|O_CLOEXEC|O_NOFOLLOW);
-        if (fd < 0)
+        if (fd < 0) {
+                if (errno == ENOENT)
+                        return 0;
                 return -errno;
+        }
 
         /* This takes possession of fd and closes it */
         return find_symlinks_fd(name, fd, config_path, config_path, same_name_link);
diff --git a/src/test/test-unit-file.c b/src/test/test-unit-file.c
index b390c44..95e2b68 100644
--- a/src/test/test-unit-file.c
+++ b/src/test/test-unit-file.c
@@ -40,7 +40,7 @@ int main(int argc, char *argv[]) {
         assert(h);
 
         r = unit_file_get_list(UNIT_FILE_SYSTEM, NULL, h);
-        log_info("%s", strerror(-r));
+        log_info("unit_file_get_list: %s", strerror(-r));
         assert(r >= 0);
 
         HASHMAP_FOREACH(p, h, i)

commit 4a271908f1d46e0549a4d9bfc5d0e34266887695
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Sun Jul 22 15:39:37 2012 +0200

    logind: redefine idleness to start at last activity
    
    Before, after the timeout, a session would be timestamped as idle
    since 'last activity' + 'idle timeout'. Now, it is timestamped as idle
    since 'last activity'.
    
    Before, after all sessions were idle, the seat would be marked with as
    idle with the timestamp of the oldest idle session. Now it is
    marked with the timestamp of the youngest idle session.
    
    Both changes seem to me to be closer to natural understanding of
    idleness: the time since last activity counts.

diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c
index c2cf6e5..0ae0c52 100644
--- a/src/login/logind-seat.c
+++ b/src/login/logind-seat.c
@@ -471,7 +471,7 @@ int seat_get_idle_hint(Seat *s, dual_timestamp *t) {
 
                 if (!ih) {
                         if (!idle_hint) {
-                                if (k.monotonic < ts.monotonic)
+                                if (k.monotonic > ts.monotonic)
                                         ts = k;
                         } else {
                                 idle_hint = false;
diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index 9740e23..f670db8 100644
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -738,7 +738,6 @@ int session_get_idle_hint(Session *s, dual_timestamp *t) {
         char *p;
         struct stat st;
         usec_t u, n;
-        bool b;
         int k;
 
         assert(s);
@@ -773,12 +772,11 @@ int session_get_idle_hint(Session *s, dual_timestamp *t) {
 
         u = timespec_load(&st.st_atim);
         n = now(CLOCK_REALTIME);
-        b = u + IDLE_THRESHOLD_USEC < n;
 
         if (t)
-                dual_timestamp_from_realtime(t, u + b*IDLE_THRESHOLD_USEC);
+                dual_timestamp_from_realtime(t, u);
 
-        return b;
+        return u + IDLE_THRESHOLD_USEC < n;
 
 dont_know:
         if (t)



More information about the systemd-commits mailing list