From 7268295765ff423a1dbbdad2db4544d3680e718a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 5 Aug 2019 15:53:28 +0200 Subject: [PATCH 1/3] update-utmp: define main() through macro Update logging a bit: drop logging of the pid, nowadays pid1 and journald do a very good job of logging that. Always log about failure to open audit fd, but at DEBUG_LEVEL if not important. --- src/update-utmp/update-utmp.c | 55 ++++++++++++++--------------------- 1 file changed, 22 insertions(+), 33 deletions(-) diff --git a/src/update-utmp/update-utmp.c b/src/update-utmp/update-utmp.c index 56fb937647..f0fc181b57 100644 --- a/src/update-utmp/update-utmp.c +++ b/src/update-utmp/update-utmp.c @@ -18,6 +18,7 @@ #include "format-util.h" #include "log.h" #include "macro.h" +#include "main-func.h" #include "process-util.h" #include "special.h" #include "strv.h" @@ -220,7 +221,7 @@ static int on_runlevel(Context *c) { return r; } -int main(int argc, char *argv[]) { +static int run(int argc, char *argv[]) { _cleanup_(context_clear) Context c = { #if HAVE_AUDIT .audit_fd = -1 @@ -228,47 +229,35 @@ int main(int argc, char *argv[]) { }; int r; - if (getppid() != 1) { - log_error("This program should be invoked by init only."); - return EXIT_FAILURE; - } - - if (argc != 2) { - log_error("This program requires one argument."); - return EXIT_FAILURE; - } + if (getppid() != 1) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), + "This program should be invoked by init only."); + if (argc != 2) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), + "This program requires one argument."); log_setup_service(); umask(0022); #if HAVE_AUDIT - /* If the kernel lacks netlink or audit support, - * don't worry about it. */ + /* If the kernel lacks netlink or audit support, don't worry about it. */ c.audit_fd = audit_open(); - if (c.audit_fd < 0 && !IN_SET(errno, EAFNOSUPPORT, EPROTONOSUPPORT)) - log_error_errno(errno, "Failed to connect to audit log: %m"); + if (c.audit_fd < 0) + log_full_errno(IN_SET(errno, EAFNOSUPPORT, EPROTONOSUPPORT) ? LOG_DEBUG : LOG_ERR, + errno, "Failed to connect to audit log: %m"); #endif r = bus_connect_system_systemd(&c.bus); - if (r < 0) { - log_error_errno(r, "Failed to get D-Bus connection: %m"); - return EXIT_FAILURE; - } - - log_debug("systemd-update-utmp running as pid "PID_FMT, getpid_cached()); + if (r < 0) + return log_error_errno(r, "Failed to get D-Bus connection: %m"); if (streq(argv[1], "reboot")) - r = on_reboot(&c); - else if (streq(argv[1], "shutdown")) - r = on_shutdown(&c); - else if (streq(argv[1], "runlevel")) - r = on_runlevel(&c); - else { - log_error("Unknown command %s", argv[1]); - return EXIT_FAILURE; - } - - log_debug("systemd-update-utmp stopped as pid "PID_FMT, getpid_cached()); - - return r < 0 ? EXIT_FAILURE : EXIT_SUCCESS; + return on_reboot(&c); + if (streq(argv[1], "shutdown")) + return on_shutdown(&c); + if (streq(argv[1], "runlevel")) + return on_runlevel(&c); + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Unknown command %s", argv[1]); } + +DEFINE_MAIN_FUNCTION(run); From 48da02ec6f46c93b083d0790e031d8f73b877858 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 6 Aug 2019 20:22:23 +0200 Subject: [PATCH 2/3] core/mount-setup: use conf_files_list_strv() for relabel-extra.d/ --- src/core/mount-setup.c | 57 +++++++++++------------------------------- 1 file changed, 15 insertions(+), 42 deletions(-) diff --git a/src/core/mount-setup.c b/src/core/mount-setup.c index ea7b0a80cb..0ec90c5aeb 100644 --- a/src/core/mount-setup.c +++ b/src/core/mount-setup.c @@ -10,8 +10,8 @@ #include "alloc-util.h" #include "bus-util.h" #include "cgroup-util.h" +#include "conf-files.h" #include "dev-setup.h" -#include "dirent-util.h" #include "efivars.h" #include "fd-util.h" #include "fileio.h" @@ -408,7 +408,8 @@ static int relabel_cgroup_filesystems(void) { } static int relabel_extra(void) { - _cleanup_closedir_ DIR *d = NULL; + _cleanup_strv_free_ char **files = NULL; + char **file; int r, c = 0; /* Support for relabelling additional files or directories after loading the policy. For this, code in the @@ -419,55 +420,27 @@ static int relabel_extra(void) { * possible. */ - d = opendir("/run/systemd/relabel-extra.d/"); - if (!d) { - if (errno == ENOENT) - return 0; + r = conf_files_list(&files, ".relabel", NULL, + CONF_FILES_FILTER_MASKED | CONF_FILES_REGULAR, + "/run/systemd/relabel-extra.d/", NULL); + if (r < 0) + return log_error_errno(r, "Failed to enumerate /run/systemd/relabel-extra.d/, ignoring: %m"); - return log_warning_errno(errno, "Failed to open /run/systemd/relabel-extra.d/, ignoring: %m"); - } - - for (;;) { + STRV_FOREACH(file, files) { _cleanup_fclose_ FILE *f = NULL; - _cleanup_close_ int fd = -1; - struct dirent *de; - errno = 0; - de = readdir_no_dot(d); - if (!de) { - if (errno != 0) - return log_error_errno(errno, "Failed read directory /run/systemd/relabel-extra.d/, ignoring: %m"); - break; - } - - if (hidden_or_backup_file(de->d_name)) - continue; - - if (!endswith(de->d_name, ".relabel")) - continue; - - if (!IN_SET(de->d_type, DT_REG, DT_UNKNOWN)) - continue; - - fd = openat(dirfd(d), de->d_name, O_RDONLY|O_CLOEXEC|O_NONBLOCK); - if (fd < 0) { - log_warning_errno(errno, "Failed to open /run/systemd/relabel-extra.d/%s, ignoring: %m", de->d_name); - continue; - } - - f = fdopen(fd, "r"); + f = fopen(*file, "re"); if (!f) { - log_warning_errno(errno, "Failed to convert file descriptor into file object, ignoring: %m"); + log_warning_errno(errno, "Failed to open %s, ignoring: %m", *file); continue; } - TAKE_FD(fd); for (;;) { _cleanup_free_ char *line = NULL; r = read_line(f, LONG_LINE_MAX, &line); if (r < 0) { - log_warning_errno(r, "Failed to read from /run/systemd/relabel-extra.d/%s, ignoring: %m", de->d_name); + log_warning_errno(r, "Failed to read %s, ignoring: %m", *file); break; } if (r == 0) /* EOF */ @@ -490,11 +463,11 @@ static int relabel_extra(void) { c++; } - if (unlinkat(dirfd(d), de->d_name, 0) < 0) - log_warning_errno(errno, "Failed to remove /run/systemd/relabel-extra.d/%s, ignoring: %m", de->d_name); + if (unlink(*file) < 0) + log_warning_errno(errno, "Failed to remove %s, ignoring: %m", *file); } - /* Remove when we completing things. */ + /* Remove when we complete things. */ if (rmdir("/run/systemd/relabel-extra.d") < 0) log_warning_errno(errno, "Failed to remove /run/systemd/relabel-extra.d/ directory: %m"); From 36b12282e1b68b963a3df1d2e6e0597a0847f792 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 19 Aug 2019 16:31:27 +0200 Subject: [PATCH 3/3] basic/conf-files: make conf_files_list() take just a single directory This function had two users (apart from tests), and both only used one argument. And it seems likely that if we need to pass more directories, either the _nulstr() or the _strv() form would be used. Let's simplify the code. --- src/basic/conf-files.c | 8 ++------ src/basic/conf-files.h | 2 +- src/core/mount-setup.c | 2 +- src/shared/bootspec.c | 2 +- src/test/test-conf-files.c | 30 +++++++++++++----------------- 5 files changed, 18 insertions(+), 26 deletions(-) diff --git a/src/basic/conf-files.c b/src/basic/conf-files.c index 3f36b5cc34..de44f45f53 100644 --- a/src/basic/conf-files.c +++ b/src/basic/conf-files.c @@ -261,16 +261,12 @@ int conf_files_list_strv(char ***strv, const char *suffix, const char *root, uns return conf_files_list_strv_internal(strv, suffix, root, flags, copy); } -int conf_files_list(char ***strv, const char *suffix, const char *root, unsigned flags, const char *dir, ...) { +int conf_files_list(char ***strv, const char *suffix, const char *root, unsigned flags, const char *dir) { _cleanup_strv_free_ char **dirs = NULL; - va_list ap; assert(strv); - va_start(ap, dir); - dirs = strv_new_ap(dir, ap); - va_end(ap); - + dirs = strv_new(dir); if (!dirs) return -ENOMEM; diff --git a/src/basic/conf-files.h b/src/basic/conf-files.h index 55ab326402..7d7be5c90f 100644 --- a/src/basic/conf-files.h +++ b/src/basic/conf-files.h @@ -11,7 +11,7 @@ enum { CONF_FILES_FILTER_MASKED = 1 << 4, }; -int conf_files_list(char ***ret, const char *suffix, const char *root, unsigned flags, const char *dir, ...) _sentinel_; +int conf_files_list(char ***ret, const char *suffix, const char *root, unsigned flags, const char *dir); int conf_files_list_strv(char ***ret, const char *suffix, const char *root, unsigned flags, const char* const* dirs); int conf_files_list_nulstr(char ***ret, const char *suffix, const char *root, unsigned flags, const char *dirs); int conf_files_insert(char ***strv, const char *root, char **dirs, const char *path); diff --git a/src/core/mount-setup.c b/src/core/mount-setup.c index 0ec90c5aeb..7ee59b2da6 100644 --- a/src/core/mount-setup.c +++ b/src/core/mount-setup.c @@ -422,7 +422,7 @@ static int relabel_extra(void) { r = conf_files_list(&files, ".relabel", NULL, CONF_FILES_FILTER_MASKED | CONF_FILES_REGULAR, - "/run/systemd/relabel-extra.d/", NULL); + "/run/systemd/relabel-extra.d/"); if (r < 0) return log_error_errno(r, "Failed to enumerate /run/systemd/relabel-extra.d/, ignoring: %m"); diff --git a/src/shared/bootspec.c b/src/shared/bootspec.c index b0c6e4885f..3b4c99e9ee 100644 --- a/src/shared/bootspec.c +++ b/src/shared/bootspec.c @@ -257,7 +257,7 @@ static int boot_entries_find( assert(entries); assert(n_entries); - r = conf_files_list(&files, ".conf", NULL, 0, dir, NULL); + r = conf_files_list(&files, ".conf", NULL, 0, dir); if (r < 0) return log_error_errno(r, "Failed to list files in \"%s\": %m", dir); diff --git a/src/test/test-conf-files.c b/src/test/test-conf-files.c index 61811311f4..f37f32f1b5 100644 --- a/src/test/test-conf-files.c +++ b/src/test/test-conf-files.c @@ -42,38 +42,34 @@ static void setup_test_dir(char *tmp_dir, const char *files, ...) { static void test_conf_files_list(bool use_root) { char tmp_dir[] = "/tmp/test-conf-files-XXXXXX"; _cleanup_strv_free_ char **found_files = NULL, **found_files2 = NULL; - const char *root_dir, *search_1, *search_2, *expect_a, *expect_b, *expect_c, *mask; + const char *root_dir, *search, *expect_a, *expect_b, *expect_c, *mask; - log_debug("/* %s(%s) */", __func__, yes_no(use_root)); + log_info("/* %s(%s) */", __func__, yes_no(use_root)); setup_test_dir(tmp_dir, - "/dir1/a.conf", - "/dir2/a.conf", - "/dir2/b.conf", - "/dir2/c.foo", - "/dir2/d.conf", + "/dir/a.conf", + "/dir/b.conf", + "/dir/c.foo", NULL); - mask = strjoina(tmp_dir, "/dir1/d.conf"); + mask = strjoina(tmp_dir, "/dir/d.conf"); assert_se(symlink("/dev/null", mask) >= 0); if (use_root) { root_dir = tmp_dir; - search_1 = "/dir1"; - search_2 = "/dir2"; + search = "/dir"; } else { root_dir = NULL; - search_1 = strjoina(tmp_dir, "/dir1"); - search_2 = strjoina(tmp_dir, "/dir2"); + search = strjoina(tmp_dir, "/dir"); } - expect_a = strjoina(tmp_dir, "/dir1/a.conf"); - expect_b = strjoina(tmp_dir, "/dir2/b.conf"); - expect_c = strjoina(tmp_dir, "/dir2/c.foo"); + expect_a = strjoina(tmp_dir, "/dir/a.conf"); + expect_b = strjoina(tmp_dir, "/dir/b.conf"); + expect_c = strjoina(tmp_dir, "/dir/c.foo"); log_debug("/* Check when filtered by suffix */"); - assert_se(conf_files_list(&found_files, ".conf", root_dir, CONF_FILES_FILTER_MASKED, search_1, search_2, NULL) == 0); + assert_se(conf_files_list(&found_files, ".conf", root_dir, CONF_FILES_FILTER_MASKED, search) == 0); strv_print(found_files); assert_se(found_files); @@ -82,7 +78,7 @@ static void test_conf_files_list(bool use_root) { assert_se(!found_files[2]); log_debug("/* Check when unfiltered */"); - assert_se(conf_files_list(&found_files2, NULL, root_dir, CONF_FILES_FILTER_MASKED, search_1, search_2, NULL) == 0); + assert_se(conf_files_list(&found_files2, NULL, root_dir, CONF_FILES_FILTER_MASKED, search) == 0); strv_print(found_files2); assert_se(found_files2);