From 49e87292dc1fa942928a1e4f10c20b9f830bf2dc Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 24 Jan 2018 10:54:10 +0100 Subject: [PATCH 1/3] tmpfiles: make "f" lines behaviour match what the documentation says MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CHANGE OF BEHAVIOUR — with this commit "f" line's behaviour is altered to match what the documentation says: if an "argument" string is specified it is written to the file only when the file didn't exist before. Previously, it would be appended to the file each time systemd-tmpfiles was invoked — which is not a particularly useful behaviour as the tool is not idempotent then and the indicated files grow without bounds each time the tool is invoked. I did some spelunking whether this change in behaviour would break things, but afaics nothing relies on the previous O_APPEND behaviour of this line type, hence I think it's relatively safe to make "f" lines work the way the docs say, rather than adding a new modifier for it or so. Triggered by: https://lists.freedesktop.org/archives/systemd-devel/2018-January/040171.html --- NEWS | 12 ++++++++++++ man/tmpfiles.d.xml | 28 ++++++++++------------------ src/tmpfiles/tmpfiles.c | 9 +++++++-- 3 files changed, 29 insertions(+), 20 deletions(-) diff --git a/NEWS b/NEWS index 46b8e12fb9..70014888db 100644 --- a/NEWS +++ b/NEWS @@ -7,6 +7,18 @@ CHANGES WITH 237 in spe: keycodes are not recognized by any major desktop. They now produce Up/Down key events so that they can be used for scrolling. + * INCOMPATIBILITY: systemd-tmpfiles' "f" lines changed behaviour + slightly: previously, if an argument was specified for lines of this + type (i.e. the right-most column was set) this string was appended to + existing files each time systemd-tmpfiles was run. This behaviour was + different from what the documentation said, and not particularly + useful, as repeated systemd-tmpfiles invocations would not be + idempotent and grow such files without bounds. With this release + behaviour has been altered slightly, to match what the documentation + says: lines of this type only have an effect if the indicated files + don't exist yet, and only then the argument string is written to the + file. + — Berlin, 2018-XX-XX CHANGES WITH 236: diff --git a/man/tmpfiles.d.xml b/man/tmpfiles.d.xml index 30aa886388..c3a43d36e4 100644 --- a/man/tmpfiles.d.xml +++ b/man/tmpfiles.d.xml @@ -147,9 +147,8 @@ L /tmp/foobar - - - - /dev/null f - Create a file if it does not exist yet. If - the argument parameter is given, it will be written to the - file. Does not follow symlinks. + Create a file if it does not exist yet. If the argument parameter is given and the file did + not exist yet, it will be written to the file. Does not follow symlinks. @@ -585,21 +584,14 @@ r! /tmp/.X[0-9]*-lock Argument - For L lines determines the destination - path of the symlink. For c and - b, determines the major/minor of the device - node, with major and minor formatted as integers, separated by - :, e.g. 1:3. For - f, F, and - w, the argument may be used to specify a short string that - is written to the file, suffixed by a newline. For - C, specifies the source file or - directory. For t and T, - determines extended attributes to be set. For - a and A, determines ACL - attributes to be set. For h and - H, determines the file attributes to - set. Ignored for all other lines. + For L lines determines the destination path of the symlink. For c and + b, determines the major/minor of the device node, with major and minor formatted as integers, + separated by :, e.g. 1:3. For f, F, + and w, the argument may be used to specify a short string that is written to the file, + suffixed by a newline. For C, specifies the source file or directory. For t + and T, determines extended attributes to be set. For a and + A, determines ACL attributes to be set. For h and H, + determines the file attributes to set. Ignored for all other lines. This field can contain specifiers, see below. diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index 4d8c36870c..38cbb739c0 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -1189,7 +1189,7 @@ static int write_one_file(Item *i, const char *path) { assert(i); assert(path); - flags = i->type == CREATE_FILE ? O_CREAT|O_APPEND|O_NOFOLLOW : + flags = i->type == CREATE_FILE ? O_CREAT|O_EXCL|O_NOFOLLOW : i->type == TRUNCATE_FILE ? O_CREAT|O_TRUNC|O_NOFOLLOW : 0; RUN_WITH_UMASK(0000) { @@ -1200,9 +1200,13 @@ static int write_one_file(Item *i, const char *path) { if (fd < 0) { if (i->type == WRITE_FILE && errno == ENOENT) { - log_debug_errno(errno, "Not writing \"%s\": %m", path); + log_debug_errno(errno, "Not writing missing file \"%s\": %m", path); return 0; } + if (i->type == CREATE_FILE && errno == EEXIST) { + log_debug_errno(errno, "Not writing to pre-existing file \"%s\": %m", path); + goto done; + } r = -errno; if (!i->argument && errno == EROFS && stat(path, &st) == 0 && @@ -1223,6 +1227,7 @@ static int write_one_file(Item *i, const char *path) { fd = safe_close(fd); +done: if (stat(path, &st) < 0) return log_error_errno(errno, "stat(%s) failed: %m", path); From 81b1dc271ff09e65b7f0d8212162e4d67d02064d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 24 Jan 2018 11:07:47 +0100 Subject: [PATCH 2/3] coccinelle: let's use STRLEN() rather strlen() at one more place --- src/shared/bus-unit-util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index 94ee71697e..bc77c3abdb 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -1382,7 +1382,7 @@ static int bus_append_socket_property(sd_bus_message *m, const char *field, cons if (isempty(eq)) r = sd_bus_message_append(m, "(sv)", "Listen", "a(ss)", 0); else - r = sd_bus_message_append(m, "(sv)", "Listen", "a(ss)", 1, field + strlen("Listen"), eq); + r = sd_bus_message_append(m, "(sv)", "Listen", "a(ss)", 1, field + STRLEN("Listen"), eq); if (r < 0) return bus_log_create_error(r); From db4a47e9fec86a3095129ce4b062b6ec81933eab Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 24 Jan 2018 11:09:29 +0100 Subject: [PATCH 3/3] =?UTF-8?q?coccinelle:=20O=5FNDELAY=20=E2=86=92=20O=5F?= =?UTF-8?q?NONBLOCK?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Apparently O_NONBLOCK is the modern name used in most documentation and for most cases in our sources. Let's hence replace the old alias O_NDELAY and stick to O_NONBLOCK everywhere. --- coccinelle/o-ndelay.occi | 4 ++++ src/journal/journald-server.c | 3 ++- src/login/logind-inhibit.c | 4 ++-- src/login/logind-session.c | 4 ++-- src/resolve/resolved-manager.c | 3 ++- src/shared/utmp-wtmp.c | 2 +- src/systemctl/systemctl.c | 2 +- 7 files changed, 14 insertions(+), 8 deletions(-) create mode 100644 coccinelle/o-ndelay.occi diff --git a/coccinelle/o-ndelay.occi b/coccinelle/o-ndelay.occi new file mode 100644 index 0000000000..669424a054 --- /dev/null +++ b/coccinelle/o-ndelay.occi @@ -0,0 +1,4 @@ +@@ +@@ +- O_NDELAY ++ O_NONBLOCK diff --git a/src/journal/journald-server.c b/src/journal/journald-server.c index 74536e1715..5cd58e8a77 100644 --- a/src/journal/journald-server.c +++ b/src/journal/journald-server.c @@ -1497,7 +1497,8 @@ static int server_open_hostname(Server *s) { assert(s); - s->hostname_fd = open("/proc/sys/kernel/hostname", O_RDONLY|O_CLOEXEC|O_NDELAY|O_NOCTTY); + s->hostname_fd = open("/proc/sys/kernel/hostname", + O_RDONLY|O_CLOEXEC|O_NONBLOCK|O_NOCTTY); if (s->hostname_fd < 0) return log_error_errno(errno, "Failed to open /proc/sys/kernel/hostname: %m"); diff --git a/src/login/logind-inhibit.c b/src/login/logind-inhibit.c index 8a6487ea45..e14835292e 100644 --- a/src/login/logind-inhibit.c +++ b/src/login/logind-inhibit.c @@ -305,7 +305,7 @@ int inhibitor_create_fifo(Inhibitor *i) { /* Open reading side */ if (i->fifo_fd < 0) { - i->fifo_fd = open(i->fifo_path, O_RDONLY|O_CLOEXEC|O_NDELAY); + i->fifo_fd = open(i->fifo_path, O_RDONLY|O_CLOEXEC|O_NONBLOCK); if (i->fifo_fd < 0) return -errno; } @@ -321,7 +321,7 @@ int inhibitor_create_fifo(Inhibitor *i) { } /* Open writing side */ - r = open(i->fifo_path, O_WRONLY|O_CLOEXEC|O_NDELAY); + r = open(i->fifo_path, O_WRONLY|O_CLOEXEC|O_NONBLOCK); if (r < 0) return -errno; diff --git a/src/login/logind-session.c b/src/login/logind-session.c index c4bde80c0c..92eb2943fe 100644 --- a/src/login/logind-session.c +++ b/src/login/logind-session.c @@ -962,7 +962,7 @@ int session_create_fifo(Session *s) { /* Open reading side */ if (s->fifo_fd < 0) { - s->fifo_fd = open(s->fifo_path, O_RDONLY|O_CLOEXEC|O_NDELAY); + s->fifo_fd = open(s->fifo_path, O_RDONLY|O_CLOEXEC|O_NONBLOCK); if (s->fifo_fd < 0) return -errno; @@ -981,7 +981,7 @@ int session_create_fifo(Session *s) { } /* Open writing side */ - r = open(s->fifo_path, O_WRONLY|O_CLOEXEC|O_NDELAY); + r = open(s->fifo_path, O_WRONLY|O_CLOEXEC|O_NONBLOCK); if (r < 0) return -errno; diff --git a/src/resolve/resolved-manager.c b/src/resolve/resolved-manager.c index 1311f04f75..2ee027791a 100644 --- a/src/resolve/resolved-manager.c +++ b/src/resolve/resolved-manager.c @@ -488,7 +488,8 @@ static int manager_watch_hostname(Manager *m) { assert(m); - m->hostname_fd = open("/proc/sys/kernel/hostname", O_RDONLY|O_CLOEXEC|O_NDELAY|O_NOCTTY); + m->hostname_fd = open("/proc/sys/kernel/hostname", + O_RDONLY|O_CLOEXEC|O_NONBLOCK|O_NOCTTY); if (m->hostname_fd < 0) { log_warning_errno(errno, "Failed to watch hostname: %m"); return 0; diff --git a/src/shared/utmp-wtmp.c b/src/shared/utmp-wtmp.c index 1715c0fb24..cab1cd6a2d 100644 --- a/src/shared/utmp-wtmp.c +++ b/src/shared/utmp-wtmp.c @@ -330,7 +330,7 @@ static int write_to_terminal(const char *tty, const char *message) { assert(tty); assert(message); - fd = open(tty, O_WRONLY|O_NDELAY|O_NOCTTY|O_CLOEXEC); + fd = open(tty, O_WRONLY|O_NONBLOCK|O_NOCTTY|O_CLOEXEC); if (fd < 0 || !isatty(fd)) return -errno; diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index adfb1f01fe..30077d319d 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -8352,7 +8352,7 @@ static int talk_initctl(void) { request.runlevel = rl; - fd = open(INIT_FIFO, O_WRONLY|O_NDELAY|O_CLOEXEC|O_NOCTTY); + fd = open(INIT_FIFO, O_WRONLY|O_NONBLOCK|O_CLOEXEC|O_NOCTTY); if (fd < 0) { if (errno == ENOENT) return 0;