[systemd-commits] 3 commits - src/core src/tmpfiles

Zbigniew Jędrzejewski-Szmek zbyszek at kemper.freedesktop.org
Sun Mar 3 17:19:46 PST 2013


 src/core/path.c         |   78 +++++++++++++++++++++++++++++-------------------
 src/core/service.c      |   16 ++++++++-
 src/tmpfiles/tmpfiles.c |   68 +++++++++++------------------------------
 3 files changed, 80 insertions(+), 82 deletions(-)

New commits:
commit 19fbec19192998fa7fc0c24a58038b98cf989802
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Sun Mar 3 18:42:52 2013 -0500

    tmpfiles: use cleanup func. to save a few lines

diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
index 7da94ed..b93d898 100644
--- a/src/tmpfiles/tmpfiles.c
+++ b/src/tmpfiles/tmpfiles.c
@@ -134,7 +134,7 @@ static struct Item* find_glob(Hashmap *h, const char *match) {
 }
 
 static void load_unix_sockets(void) {
-        FILE *f = NULL;
+        FILE _cleanup_fclose_ *f = NULL;
         char line[LINE_MAX];
 
         if (unix_sockets)
@@ -194,15 +194,11 @@ static void load_unix_sockets(void) {
                 }
         }
 
-        fclose(f);
         return;
 
 fail:
         set_free_free(unix_sockets);
         unix_sockets = NULL;
-
-        if (f)
-                fclose(f);
 }
 
 static bool unix_socket_alive(const char *fn) {
@@ -231,12 +227,12 @@ static int dir_cleanup(
         struct dirent *dent;
         struct timespec times[2];
         bool deleted = false;
-        char *sub_path = NULL;
         int r = 0;
 
         while ((dent = readdir(d))) {
                 struct stat s;
                 usec_t age;
+                char _cleanup_free_ *sub_path = NULL;
 
                 if (streq(dent->d_name, ".") ||
                     streq(dent->d_name, ".."))
@@ -260,9 +256,6 @@ static int dir_cleanup(
                 if (s.st_uid == 0 && !(s.st_mode & S_IWUSR))
                         continue;
 
-                free(sub_path);
-                sub_path = NULL;
-
                 if (asprintf(&sub_path, "%s/%s", p, dent->d_name) < 0) {
                         r = log_oom();
                         goto finish;
@@ -285,7 +278,7 @@ static int dir_cleanup(
                         if (maxdepth <= 0)
                                 log_warning("Reached max depth on %s.", sub_path);
                         else {
-                                DIR *sub_dir;
+                                DIR _cleanup_closedir_ *sub_dir;
                                 int q;
 
                                 sub_dir = xopendirat(dirfd(d), dent->d_name, O_NOFOLLOW|O_NOATIME);
@@ -299,7 +292,6 @@ static int dir_cleanup(
                                 }
 
                                 q = dir_cleanup(i, sub_path, sub_dir, &s, cutoff, rootdev, false, maxdepth-1, false);
-                                closedir(sub_dir);
 
                                 if (q < 0)
                                         r = q;
@@ -393,8 +385,6 @@ finish:
                         log_error("utimensat(%s): %m", p);
         }
 
-        free(sub_path);
-
         return r;
 }
 
@@ -483,7 +473,7 @@ static int write_one_file(Item *i, const char *path) {
 }
 
 static int recursive_relabel_children(Item *i, const char *path) {
-        DIR *d;
+        DIR _cleanup_closedir_ *d;
         int ret = 0;
 
         /* This returns the first error we run into, but nevertheless
@@ -498,7 +488,7 @@ static int recursive_relabel_children(Item *i, const char *path) {
                 union dirent_storage buf;
                 bool is_dir;
                 int r;
-                char *entry_path;
+                char _cleanup_free_ *entry_path = NULL;
 
                 r = readdir_r(d, &buf.de, &de);
                 if (r != 0) {
@@ -525,7 +515,6 @@ static int recursive_relabel_children(Item *i, const char *path) {
                         if (lstat(entry_path, &st) < 0) {
                                 if (ret == 0 && errno != ENOENT)
                                         ret = -errno;
-                                free(entry_path);
                                 continue;
                         }
 
@@ -538,7 +527,6 @@ static int recursive_relabel_children(Item *i, const char *path) {
                 if (r < 0) {
                         if (ret == 0 && r != -ENOENT)
                                 ret = r;
-                        free(entry_path);
                         continue;
                 }
 
@@ -547,12 +535,8 @@ static int recursive_relabel_children(Item *i, const char *path) {
                         if (r < 0 && ret == 0)
                                 ret = r;
                 }
-
-                free(entry_path);
         }
 
-        closedir(d);
-
         return ret;
 }
 
@@ -856,7 +840,7 @@ static int remove_item(Item *i) {
 }
 
 static int clean_item_instance(Item *i, const char* instance) {
-        DIR *d;
+        DIR _cleanup_closedir_ *d = NULL;
         struct stat s, ps;
         bool mountpoint;
         int r;
@@ -884,31 +868,24 @@ static int clean_item_instance(Item *i, const char* instance) {
 
         if (fstat(dirfd(d), &s) < 0) {
                 log_error("stat(%s) failed: %m", i->path);
-                r = -errno;
-                goto finish;
+                return -errno;
         }
 
         if (!S_ISDIR(s.st_mode)) {
                 log_error("%s is not a directory.", i->path);
-                r = -ENOTDIR;
-                goto finish;
+                return -ENOTDIR;
         }
 
         if (fstatat(dirfd(d), "..", &ps, AT_SYMLINK_NOFOLLOW) != 0) {
                 log_error("stat(%s/..) failed: %m", i->path);
-                r = -errno;
-                goto finish;
+                return -errno;
         }
 
         mountpoint = s.st_dev != ps.st_dev ||
                      (s.st_dev == ps.st_dev && s.st_ino == ps.st_ino);
 
-        r = dir_cleanup(i, instance, d, &s, cutoff, s.st_dev, mountpoint, MAX_DEPTH, i->keep_first_level);
-
-finish:
-        if (d)
-                closedir(d);
-
+        r = dir_cleanup(i, instance, d, &s, cutoff, s.st_dev, mountpoint,
+                        MAX_DEPTH, i->keep_first_level);
         return r;
 }
 
@@ -1002,7 +979,8 @@ static bool item_equal(Item *a, Item *b) {
 
 static int parse_line(const char *fname, unsigned line, const char *buffer) {
         Item *i, *existing;
-        char *mode = NULL, *user = NULL, *group = NULL, *age = NULL;
+        char _cleanup_free_
+                *mode = NULL, *user = NULL, *group = NULL, *age = NULL;
         char type;
         Hashmap *h;
         int r, n = -1;
@@ -1015,21 +993,16 @@ static int parse_line(const char *fname, unsigned line, const char *buffer) {
         if (!i)
                 return log_oom();
 
-        if (sscanf(buffer,
-                   "%c "
-                   "%ms "
-                   "%ms "
-                   "%ms "
-                   "%ms "
-                   "%ms "
-                   "%n",
+        r = sscanf(buffer,
+                   "%c %ms %ms %ms %ms %ms %n",
                    &type,
                    &i->path,
                    &mode,
                    &user,
                    &group,
                    &age,
-                   &n) < 2) {
+                   &n);
+        if (r < 2) {
                 log_error("[%s:%u] Syntax error.", fname, line);
                 r = -EIO;
                 goto finish;
@@ -1196,11 +1169,6 @@ static int parse_line(const char *fname, unsigned line, const char *buffer) {
         r = 0;
 
 finish:
-        free(user);
-        free(group);
-        free(mode);
-        free(age);
-
         if (i)
                 item_free(i);
 

commit a6187d4ce884e3c9ccf18d46b2ee494af386cb42
Author: Lukas Nykryn <lnykryn at redhat.com>
Date:   Fri Mar 1 18:29:59 2013 +0100

    tmpfiles: move exclamation mark into right place
    
    Unary not has higher precedence than comparisons,
    so the condition was bogus.

diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
index 6b3f70e..7da94ed 100644
--- a/src/tmpfiles/tmpfiles.c
+++ b/src/tmpfiles/tmpfiles.c
@@ -321,7 +321,7 @@ static int dir_cleanup(
                         if (age >= cutoff)
                                 continue;
 
-                        if (!i->type == IGNORE_DIRECTORY_PATH || !streq(dent->d_name, p)) {
+                        if (i->type != IGNORE_DIRECTORY_PATH || !streq(dent->d_name, p)) {
                                 log_debug("rmdir '%s'\n", sub_path);
 
                                 if (unlinkat(dirfd(d), dent->d_name, AT_REMOVEDIR) < 0) {

commit bc41f93e90f6edcc9067f3bc1085bb6c85082c00
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Sun Mar 3 01:32:34 2013 -0500

    core/path: install inotify watches top-down instead of bottom-up
    
    When watches are installed from the bottom, it is always possible
    to race, and miss a file creation event. The race can be avoided
    if a watch is first established for a parent directory, and then for
    the file in the directory. If the file is created in the time between,
    the watch on the parent directory will fire.
    
    Some messages (mostly at debug level) are added to help diagnose
    pidfile issues.
    
    Should fix https://bugzilla.redhat.com/show_bug.cgi?id=917075.

diff --git a/src/core/path.c b/src/core/path.c
index fc10128..01a2b08 100644
--- a/src/core/path.c
+++ b/src/core/path.c
@@ -53,8 +53,8 @@ int path_spec_watch(PathSpec *s, Unit *u) {
         };
 
         bool exists = false;
-        char _cleanup_free_ *k = NULL;
-        char *slash;
+        char _cleanup_free_ *path = NULL;
+        char *slash, *oldslash = NULL;
         int r;
 
         assert(u);
@@ -62,8 +62,8 @@ int path_spec_watch(PathSpec *s, Unit *u) {
 
         path_spec_unwatch(s, u);
 
-        k = strdup(s->path);
-        if (!k)
+        path = strdup(s->path);
+        if (!path)
                 return -ENOMEM;
 
         s->inotify_fd = inotify_init1(IN_NONBLOCK|IN_CLOEXEC);
@@ -76,43 +76,61 @@ int path_spec_watch(PathSpec *s, Unit *u) {
         if (r < 0)
                 goto fail;
 
-        s->primary_wd = inotify_add_watch(s->inotify_fd, k, flags_table[s->type]);
-        if (s->primary_wd >= 0)
-                exists = true;
-        else if (errno != EACCES && errno != ENOENT) {
-                log_error("Failed to add watch on %s: %m", k);
-                r = -errno;
-                goto fail;
-        }
+        /* This assumes the path was passed through path_kill_slashes()! */
 
-        do {
+        for(slash = strchr(path, '/'); ; slash = strchr(slash+1, '/')) {
                 int flags;
+                char tmp;
+
+                if (slash) {
+                        tmp = slash[slash == path];
+                        slash[slash == path] = '\0';
+                        flags = IN_MOVE_SELF | IN_DELETE_SELF | IN_ATTRIB | IN_CREATE | IN_MOVED_TO;
+                } else {
+                        flags = flags_table[s->type];
+                }
 
-                /* This assumes the path was passed through path_kill_slashes()! */
-                slash = strrchr(k, '/');
-                if (!slash)
-                        break;
-
-                /* Trim the path at the last slash. Keep the slash if it's the root dir. */
-                slash[slash == k] = 0;
-
-                flags = IN_MOVE_SELF;
-                if (!exists)
-                        flags |= IN_DELETE_SELF | IN_ATTRIB | IN_CREATE | IN_MOVED_TO;
+                r = inotify_add_watch(s->inotify_fd, path, flags);
+                if (r < 0) {
+                        if (errno == EACCES || errno == ENOENT)
+                                break;
 
-                if (inotify_add_watch(s->inotify_fd, k, flags) >= 0)
-                        exists = true;
-                else if (errno != EACCES && errno != ENOENT) {
-                        log_error("Failed to add watch on %s: %m", k);
+                        log_warning("Failed to add watch on %s: %m", path);
                         r = -errno;
                         goto fail;
+                } else {
+                        exists = true;
+
+                        /* Path exists, we don't need to watch parent
+                           too closely. */
+                        if (oldslash) {
+                                char tmp2 = oldslash[oldslash == path];
+                                oldslash[oldslash == path] = '\0';
+
+                                inotify_add_watch(s->inotify_fd, path, IN_MOVE_SELF);
+                                /* Error is ignored, the worst can happen is
+                                   we get spurious events. */
+
+                                oldslash[oldslash == path] = tmp2;
+                        }
                 }
-        } while (slash != k);
+
+                if (slash) {
+                        slash[slash == path] = tmp;
+                        oldslash = slash;
+                } else {
+                        /* whole path has been iterated over */
+                        s->primary_wd = r;
+                        break;
+                }
+        }
+
+        assert(errno == EACCES || errno == ENOENT || streq(path, s->path));
 
         if (!exists) {
                 log_error("Failed to add watch on any of the components of %s: %m",
                           s->path);
-                r = -errno;
+                r = -errno; /* either EACCESS or ENOENT */
                 goto fail;
         }
 
diff --git a/src/core/service.c b/src/core/service.c
index 3f8aabc..61b150c 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -1400,8 +1400,13 @@ static int service_load_pid_file(Service *s, bool may_warn) {
         }
 
         r = parse_pid(k, &pid);
-        if (r < 0)
+        if (r < 0) {
+                if (may_warn)
+                        log_info_unit(UNIT(s)->id,
+                                      "Failed to read PID from file %s: %s",
+                                      s->pid_file, strerror(-r));
                 return r;
+        }
 
         if (kill(pid, 0) < 0 && errno != EPERM) {
                 if (may_warn)
@@ -1429,9 +1434,13 @@ static int service_load_pid_file(Service *s, bool may_warn) {
                 return r;
 
         r = unit_watch_pid(UNIT(s), pid);
-        if (r < 0)
+        if (r < 0) {
                 /* FIXME: we need to do something here */
+                log_warning_unit(UNIT(s)->id,
+                                 "Failed to watch PID %lu from service %s",
+                                 (unsigned long) pid, UNIT(s)->id);
                 return r;
+        }
 
         return 0;
 }
@@ -2824,6 +2833,9 @@ static int service_watch_pid_file(Service *s) {
                 goto fail;
 
         /* the pidfile might have appeared just before we set the watch */
+        log_debug_unit(UNIT(s)->id,
+                       "Trying to read %s's PID file %s in case it changed",
+                       UNIT(s)->id, s->pid_file_pathspec->path);
         service_retry_pid_file(s);
 
         return 0;



More information about the systemd-commits mailing list