From 0c2b96fea853ce7f16a9326c3c98117d75defdf6 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 22 Mar 2018 16:53:26 +0100 Subject: [PATCH] macro: introduce TAKE_PTR() macro This macro will read a pointer of any type, return it, and set the pointer to NULL. This is useful as an explicit concept of passing ownership of a memory area between pointers. This takes inspiration from Rust: https://doc.rust-lang.org/std/option/enum.Option.html#method.take and was suggested by Alan Jenkins (@sourcejedi). It drops ~160 lines of code from our codebase, which makes me like it. Also, I think it clarifies passing of ownership, and thus helps readability a bit (at least for the initiated who know the new macro) --- src/basic/alloc-util.h | 9 ++ src/basic/cap-list.c | 3 +- src/basic/cgroup-util.c | 27 ++--- src/basic/extract-word.c | 3 +- src/basic/fileio.c | 9 +- src/basic/fs-util.c | 19 ++-- src/basic/locale-util.c | 3 +- src/basic/mount-util.c | 9 +- src/basic/parse-util.c | 3 +- src/basic/path-util.c | 3 +- src/basic/proc-cmdline.c | 6 +- src/basic/process-util.c | 5 +- src/basic/socket-util.c | 3 +- src/basic/strv.c | 6 +- src/basic/terminal-util.c | 13 +-- src/basic/unit-name.c | 3 +- src/core/cgroup.c | 5 +- src/core/mount-setup.c | 48 ++------- src/libelogind/sd-bus/bus-objects.c | 3 +- src/libelogind/sd-bus/sd-bus.c | 3 +- src/libelogind/sd-daemon/sd-daemon.c | 3 +- src/libelogind/sd-login/sd-login.c | 28 ++--- src/login/logind-seat.c | 3 +- src/login/logind-session.c | 46 +------- src/login/logind.c | 55 ++++++++-- src/shared/bus-util.c | 154 ++++++++++++++++++--------- src/shared/nsflags.c | 3 +- 27 files changed, 232 insertions(+), 243 deletions(-) diff --git a/src/basic/alloc-util.h b/src/basic/alloc-util.h index ec7808c1f..b1e0edbb7 100644 --- a/src/basic/alloc-util.h +++ b/src/basic/alloc-util.h @@ -130,3 +130,12 @@ void* greedy_realloc0(void **p, size_t *allocated, size_t need, size_t size); _new_ = alloca_align(_size_, (align)); \ (void*)memset(_new_, 0, _size_); \ }) + +/* Takes inspiration from Rusts's Option::take() method: reads and returns a pointer, but at the same time resets it to + * NULL. See: https://doc.rust-lang.org/std/option/enum.Option.html#method.take */ +#define TAKE_PTR(ptr) \ + ({ \ + typeof(ptr) _ptr_ = (ptr); \ + (ptr) = NULL; \ + _ptr_; \ + }) diff --git a/src/basic/cap-list.c b/src/basic/cap-list.c index c4557666e..9416391a5 100644 --- a/src/basic/cap-list.c +++ b/src/basic/cap-list.c @@ -103,8 +103,7 @@ int capability_set_to_string_alloc(uint64_t set, char **s) { str[n > 0 ? n - 1 : 0] = '\0'; /* truncate the last space, if it's there */ - *s = str; - str = NULL; + *s = TAKE_PTR(str); return 0; } diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c index 4fc542cc8..dbc2938c0 100644 --- a/src/basic/cgroup-util.c +++ b/src/basic/cgroup-util.c @@ -1452,10 +1452,9 @@ int cg_pid_get_path_shifted(pid_t pid, const char *root, char **cgroup) { if (r < 0) return r; - if (c == raw) { - *cgroup = raw; - raw = NULL; - } else { + if (c == raw) + *cgroup = TAKE_PTR(raw); + else { char *n; n = strdup(c); @@ -2087,6 +2086,14 @@ int cg_slice_to_path(const char *unit, char **ret) { _cleanup_free_ char *escaped = NULL; char n[dash - p + sizeof(".slice")]; +#ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION + /* msan doesn't instrument stpncpy, so it thinks + * n is later used unitialized: + * https://github.com/google/sanitizers/issues/926 + */ + zero(n); +#endif + /* Don't allow trailing or double dashes */ if (IN_SET(dash[1], 0, '-')) return -EINVAL; @@ -2112,8 +2119,7 @@ int cg_slice_to_path(const char *unit, char **ret) { if (!strextend(&s, e, NULL)) return -ENOMEM; - *ret = s; - s = NULL; + *ret = TAKE_PTR(s); return 0; } @@ -2406,8 +2412,7 @@ int cg_mask_to_string(CGroupMask mask, char **ret) { assert(s); s[n] = 0; - *ret = s; - s = NULL; + *ret = TAKE_PTR(s); return 0; } @@ -2590,7 +2595,7 @@ static int cg_unified_update(void) { return 0; if (statfs("/sys/fs/cgroup/", &fs) < 0) - return log_debug_errno(errno, "statfs(\"/sys/fs/cgroup/\" failed: %m"); + return log_debug_errno(errno, "statfs(\"/sys/fs/cgroup/\") failed: %m"); if (F_TYPE_EQUAL(fs.f_type, CGROUP2_SUPER_MAGIC)) { log_debug("Found cgroup2 on /sys/fs/cgroup/, full unified hierarchy"); @@ -2727,10 +2732,8 @@ int cg_enable_everywhere(CGroupMask supported, CGroupMask mask, const char *p) { } r = write_string_stream(f, s, 0); - if (r < 0) { + if (r < 0) log_debug_errno(r, "Failed to enable controller %s for %s (%s): %m", n, p, fs); - clearerr(f); - } } } diff --git a/src/basic/extract-word.c b/src/basic/extract-word.c index b900d7e19..4bbd78ac5 100644 --- a/src/basic/extract-word.c +++ b/src/basic/extract-word.c @@ -194,8 +194,7 @@ finish: finish_force_next: s[sz] = 0; - *ret = s; - s = NULL; + *ret = TAKE_PTR(s); return 1; } diff --git a/src/basic/fileio.c b/src/basic/fileio.c index 2e8991925..dd8fb1fcb 100644 --- a/src/basic/fileio.c +++ b/src/basic/fileio.c @@ -330,8 +330,7 @@ int read_full_stream(FILE *f, char **contents, size_t *size) { } buf[l] = 0; - *contents = buf; - buf = NULL; /* do not free */ + *contents = TAKE_PTR(buf); if (size) *size = l; @@ -1441,8 +1440,7 @@ int open_tmpfile_linkable(const char *target, int flags, char **ret_path) { if (fd < 0) return -errno; - *ret_path = tmp; - tmp = NULL; + *ret_path = TAKE_PTR(tmp); return fd; } @@ -1530,8 +1528,7 @@ int read_nul_string(FILE *f, char **ret) { return -ENOMEM; } - *ret = x; - x = NULL; + *ret = TAKE_PTR(x); return 0; } diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c index e556ff7e0..f5764454f 100644 --- a/src/basic/fs-util.c +++ b/src/basic/fs-util.c @@ -469,10 +469,8 @@ int get_files_in_directory(const char *path, char ***list) { n++; } - if (list) { - *list = l; - l = NULL; /* avoid freeing */ - } + if (list) + *list = TAKE_PTR(l); return n; } @@ -853,10 +851,9 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags, } /* If this is not a symlink, then let's just add the name we read to what we already verified. */ - if (!done) { - done = first; - first = NULL; - } else { + if (!done) + done = TAKE_PTR(first); + else { /* If done is "/", as first also contains slash at the head, then remove this redundant slash. */ if (streq(done, "/")) *done = '\0'; @@ -878,10 +875,8 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags, return -ENOMEM; } - if (ret) { - *ret = done; - done = NULL; - } + if (ret) + *ret = TAKE_PTR(done); if (flags & CHASE_OPEN) { int q; diff --git a/src/basic/locale-util.c b/src/basic/locale-util.c index 266cb2993..de3d7c8c8 100644 --- a/src/basic/locale-util.c +++ b/src/basic/locale-util.c @@ -341,8 +341,7 @@ int get_keymaps(char ***ret) { strv_sort(l); - *ret = l; - l = NULL; + *ret = TAKE_PTR(l); return 0; } diff --git a/src/basic/mount-util.c b/src/basic/mount-util.c index db90194ff..b74d51a26 100644 --- a/src/basic/mount-util.c +++ b/src/basic/mount-util.c @@ -81,10 +81,8 @@ int name_to_handle_at_loop( if (name_to_handle_at(fd, path, h, &mnt_id, flags) >= 0) { - if (ret_handle) { - *ret_handle = h; - h = NULL; - } + if (ret_handle) + *ret_handle = TAKE_PTR(h); if (ret_mnt_id) *ret_mnt_id = mnt_id; @@ -956,8 +954,7 @@ int mount_option_mangle( } *ret_mount_flags = mount_flags; - *ret_remaining_options = ret; - ret = NULL; + *ret_remaining_options = TAKE_PTR(ret); return 0; } diff --git a/src/basic/parse-util.c b/src/basic/parse-util.c index 4c2625389..d5886b1f8 100644 --- a/src/basic/parse-util.c +++ b/src/basic/parse-util.c @@ -329,8 +329,7 @@ int parse_syscall_and_errno(const char *in, char **name, int *error) { return -EINVAL; *error = e; - *name = n; - n = NULL; + *name = TAKE_PTR(n); return 0; } diff --git a/src/basic/path-util.c b/src/basic/path-util.c index c350bc85e..aca5f486d 100644 --- a/src/basic/path-util.c +++ b/src/basic/path-util.c @@ -294,8 +294,7 @@ char **path_strv_resolve(char **l, const char *root) { r = chase_symlinks(t, root, 0, &u); if (r == -ENOENT) { if (root) { - u = orig; - orig = NULL; + u = TAKE_PTR(orig); free(t); } else u = t; diff --git a/src/basic/proc-cmdline.c b/src/basic/proc-cmdline.c index 99d00ab0f..1d1226b28 100644 --- a/src/basic/proc-cmdline.c +++ b/src/basic/proc-cmdline.c @@ -207,10 +207,8 @@ int proc_cmdline_get_key(const char *key, unsigned flags, char **value) { } } - if (value) { - *value = ret; - ret = NULL; - } + if (value) + *value = TAKE_PTR(ret); return found; } diff --git a/src/basic/process-util.c b/src/basic/process-util.c index 81c4077c8..b90272e97 100644 --- a/src/basic/process-util.c +++ b/src/basic/process-util.c @@ -626,8 +626,7 @@ int get_process_environ(pid_t pid, char **env) { } else outcome[sz] = '\0'; - *env = outcome; - outcome = NULL; + *env = TAKE_PTR(outcome); return 0; } @@ -1479,7 +1478,7 @@ static const char *const ioprio_class_table[] = { [IOPRIO_CLASS_IDLE] = "idle" }; -DEFINE_STRING_TABLE_LOOKUP_WITH_FALLBACK(ioprio_class, int, INT_MAX); +DEFINE_STRING_TABLE_LOOKUP_WITH_FALLBACK(ioprio_class, int, IOPRIO_N_CLASSES); static const char *const sigchld_code_table[] = { [CLD_EXITED] = "exited", diff --git a/src/basic/socket-util.c b/src/basic/socket-util.c index c94e2899d..23a533700 100644 --- a/src/basic/socket-util.c +++ b/src/basic/socket-util.c @@ -1011,8 +1011,7 @@ int getpeersec(int fd, char **ret) { if (isempty(s)) return -EOPNOTSUPP; - *ret = s; - s = NULL; + *ret = TAKE_PTR(s); return 0; } diff --git a/src/basic/strv.c b/src/basic/strv.c index b6f762702..2c635b951 100644 --- a/src/basic/strv.c +++ b/src/basic/strv.c @@ -345,8 +345,7 @@ int strv_split_extract(char ***t, const char *s, const char *separators, Extract if (!GREEDY_REALLOC(l, allocated, n + 2)) return -ENOMEM; - l[n++] = word; - word = NULL; + l[n++] = TAKE_PTR(word); l[n] = NULL; } @@ -357,8 +356,7 @@ int strv_split_extract(char ***t, const char *s, const char *separators, Extract return -ENOMEM; } - *t = l; - l = NULL; + *t = TAKE_PTR(l); return (int) n; } diff --git a/src/basic/terminal-util.c b/src/basic/terminal-util.c index d1d88ab93..3ae079a2b 100644 --- a/src/basic/terminal-util.c +++ b/src/basic/terminal-util.c @@ -717,10 +717,9 @@ int vtnr_from_tty(const char *tty) { tty = active; } - if (tty == active) { - *ret = active; - active = NULL; - } else { + if (tty == active) + *ret = TAKE_PTR(active); + else { char *tmp; tmp = strdup(tty); @@ -788,8 +787,7 @@ int get_kernel_consoles(char ***ret) { goto fallback; } - *ret = l; - l = NULL; + *ret = TAKE_PTR(l); return 0; @@ -798,8 +796,7 @@ fallback: if (r < 0) return r; - *ret = l; - l = NULL; + *ret = TAKE_PTR(l); return 0; } diff --git a/src/basic/unit-name.c b/src/basic/unit-name.c index 1d938f662..e94f1d6e9 100644 --- a/src/basic/unit-name.c +++ b/src/basic/unit-name.c @@ -368,8 +368,7 @@ int unit_name_unescape(const char *f, char **ret) { *t = 0; - *ret = r; - r = NULL; + *ret = TAKE_PTR(r); return 0; } diff --git a/src/core/cgroup.c b/src/core/cgroup.c index 7c590a661..7b9e29b6f 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -1382,8 +1382,7 @@ int unit_set_cgroup_path(Unit *u, const char *path) { unit_release_cgroup(u); - u->cgroup_path = p; - p = NULL; + u->cgroup_path = TAKE_PTR(p); return 1; } @@ -2341,7 +2340,7 @@ void manager_shutdown_cgroup(Manager *m, bool delete) { #if 0 /// elogind is not init /* We can't really delete the group, since we are in it. But * let's trim it. */ - if (delete && m->cgroup_root && m->test_run_flags != MANAGER_TEST_RUN_MINIMAL) + if (delete && m->cgroup_root) (void) cg_trim(SYSTEMD_CGROUP_CONTROLLER, m->cgroup_root, false); m->cgroup_empty_event_source = sd_event_source_unref(m->cgroup_empty_event_source); diff --git a/src/core/mount-setup.c b/src/core/mount-setup.c index d2dac76cf..7073175e1 100644 --- a/src/core/mount-setup.c +++ b/src/core/mount-setup.c @@ -22,7 +22,6 @@ #include #include #include -//#include #include #include "alloc-util.h" @@ -331,10 +330,8 @@ int mount_cgroup_controllers(char ***join_controllers) { options = strv_join(*k, ","); if (!options) return log_oom(); - } else { - options = controller; - controller = NULL; - } + } else + options = TAKE_PTR(controller); where = strappend("/sys/fs/cgroup/", options); if (!where) @@ -403,35 +400,6 @@ static int nftw_cb( return FTW_CONTINUE; }; - -static int relabel_cgroup_filesystems(void) { - int r; - struct statfs st; - - r = cg_all_unified(); - if (r == 0) { - /* Temporarily remount the root cgroup filesystem to give it a proper label. Do this - only when the filesystem has been already populated by a previous instance of systemd - running from initrd. Otherwise don't remount anything and leave the filesystem read-write - for the cgroup filesystems to be mounted inside. */ - r = statfs("/sys/fs/cgroup", &st); - if (r < 0) { - return log_error_errno(errno, "Failed to determine mount flags for /sys/fs/cgroup: %m"); - } - - if (st.f_flags & ST_RDONLY) - (void) mount(NULL, "/sys/fs/cgroup", NULL, MS_REMOUNT, NULL); - - label_fix("/sys/fs/cgroup", false, false); - nftw("/sys/fs/cgroup", nftw_cb, 64, FTW_MOUNT|FTW_PHYS|FTW_ACTIONRETVAL); - - if (st.f_flags & ST_RDONLY) - (void) mount(NULL, "/sys/fs/cgroup", NULL, MS_REMOUNT|MS_RDONLY, NULL); - } else if (r < 0) - return log_error_errno(r, "Failed to determine whether we are in all unified mode: %m"); - - return 0; -} #endif #endif // 0 @@ -458,9 +426,15 @@ int mount_setup(bool loaded_policy) { nftw("/dev/shm", nftw_cb, 64, FTW_MOUNT|FTW_PHYS|FTW_ACTIONRETVAL); nftw("/run", nftw_cb, 64, FTW_MOUNT|FTW_PHYS|FTW_ACTIONRETVAL); - r = relabel_cgroup_filesystems(); - if (r < 0) - return r; + /* Temporarily remount the root cgroup filesystem to give it a proper label. */ + r = cg_all_unified(); + if (r == 0) { + (void) mount(NULL, "/sys/fs/cgroup", NULL, MS_REMOUNT, NULL); + label_fix("/sys/fs/cgroup", false, false); + nftw("/sys/fs/cgroup", nftw_cb, 64, FTW_MOUNT|FTW_PHYS|FTW_ACTIONRETVAL); + (void) mount(NULL, "/sys/fs/cgroup", NULL, MS_REMOUNT|MS_RDONLY, NULL); + } else if (r < 0) + return log_error_errno(r, "Failed to determine whether we are in all unified mode: %m"); after_relabel = now(CLOCK_MONOTONIC); diff --git a/src/libelogind/sd-bus/bus-objects.c b/src/libelogind/sd-bus/bus-objects.c index 6e00255b2..08015a99e 100644 --- a/src/libelogind/sd-bus/bus-objects.c +++ b/src/libelogind/sd-bus/bus-objects.c @@ -1470,8 +1470,7 @@ static struct node *bus_node_allocate(sd_bus *bus, const char *path) { return NULL; n->parent = parent; - n->path = s; - s = NULL; /* do not free */ + n->path = TAKE_PTR(s); r = hashmap_put(bus->nodes, n->path, n); if (r < 0) { diff --git a/src/libelogind/sd-bus/sd-bus.c b/src/libelogind/sd-bus/sd-bus.c index eb415e6e3..5df3af5b4 100644 --- a/src/libelogind/sd-bus/sd-bus.c +++ b/src/libelogind/sd-bus/sd-bus.c @@ -1350,8 +1350,7 @@ int bus_set_address_user(sd_bus *b) { if (asprintf(&s, DEFAULT_USER_BUS_ADDRESS_FMT, ee) < 0) return -ENOMEM; - b->address = s; - s = NULL; + b->address = TAKE_PTR(s); return 0; } diff --git a/src/libelogind/sd-daemon/sd-daemon.c b/src/libelogind/sd-daemon/sd-daemon.c index c27ab7303..b26d387ef 100644 --- a/src/libelogind/sd-daemon/sd-daemon.c +++ b/src/libelogind/sd-daemon/sd-daemon.c @@ -144,8 +144,7 @@ _public_ int sd_listen_fds_with_names(int unset_environment, char ***names) { return r; } - *names = l; - l = NULL; + *names = TAKE_PTR(l); return n_fds; } diff --git a/src/libelogind/sd-login/sd-login.c b/src/libelogind/sd-login/sd-login.c index 53dc6bc84..490a56a94 100644 --- a/src/libelogind/sd-login/sd-login.c +++ b/src/libelogind/sd-login/sd-login.c @@ -355,8 +355,7 @@ _public_ int sd_uid_get_display(uid_t uid, char **session) { if (isempty(s)) return -ENODATA; - *session = s; - s = NULL; + *session = TAKE_PTR(s); return 0; } @@ -385,8 +384,7 @@ static int file_of_seat(const char *seat, char **_p) { if (!p) return -ENOMEM; - *_p = p; - p = NULL; + *_p = TAKE_PTR(p); return 0; } @@ -559,8 +557,7 @@ _public_ int sd_session_get_state(const char *session, char **state) { if (isempty(s)) return -EIO; - *state = s; - s = NULL; + *state = TAKE_PTR(s); return 0; } @@ -605,8 +602,7 @@ static int session_get_string(const char *session, const char *field, char **val if (isempty(s)) return -ENODATA; - *value = s; - s = NULL; + *value = TAKE_PTR(s); return 0; } @@ -711,10 +707,8 @@ _public_ int sd_seat_get_active(const char *seat, char **session, uid_t *uid) { return r; } - if (session && s) { - *session = s; - s = NULL; - } + if (session && s) + *session = TAKE_PTR(s); return 0; } @@ -939,10 +933,9 @@ _public_ int sd_get_machine_names(char ***machines) { *b = NULL; } - if (machines) { - *machines = l; - l = NULL; - } + if (machines) + *machines = TAKE_PTR(l); + return r; } @@ -963,8 +956,7 @@ _public_ int sd_machine_get_class(const char *machine, char **class) { if (!c) return -EIO; - *class = c; - c = NULL; + *class = TAKE_PTR(c); return 0; } diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c index 3b1962ddd..b109148e6 100644 --- a/src/login/logind-seat.c +++ b/src/login/logind-seat.c @@ -566,8 +566,7 @@ void seat_complete_switch(Seat *s) { if (!s->pending_switch) return; - session = s->pending_switch; - s->pending_switch = NULL; + session = TAKE_PTR(s->pending_switch); seat_set_active(s, session); } diff --git a/src/login/logind-session.c b/src/login/logind-session.c index 79669b2a8..de5ec47e4 100644 --- a/src/login/logind-session.c +++ b/src/login/logind-session.c @@ -732,21 +732,6 @@ static int session_stop_cgroup(Session *s, bool force) { assert(s); -#if 0 /// elogind must not kill lingering user processes alive - if (force || manager_shall_kill(s->manager, s->user->name)) { -#else - if ( (force || manager_shall_kill(s->manager, s->user->name) ) - && (user_check_linger_file(s->user) < 1) ) { -#endif // 1 - - r = session_kill(s, KILL_ALL, SIGTERM); - if (r < 0) - return r; - } - - return 0; -} -#endif // 0 int session_stop(Session *s, bool force) { int r; @@ -765,11 +750,7 @@ int session_stop(Session *s, bool force) { session_remove_fifo(s); /* Kill cgroup */ -#if 0 /// elogind does not start scopes, but sessions r = session_stop_scope(s, force); -#else - r = session_stop_cgroup(s, force); -#endif // 0 s->stopping = true; @@ -778,9 +759,6 @@ int session_stop(Session *s, bool force) { session_save(s); user_save(s->user); -#if 1 /// elogind must queue this session again - session_add_to_gc_queue(s); -#endif // 1 return r; } @@ -1078,12 +1056,10 @@ bool session_may_gc(Session *s, bool drop_not_started) { return false; } -#if 0 /// elogind supports neither scopes nor jobs if (s->scope_job && manager_job_is_active(s->manager, s->scope_job)) return false; if (s->scope && manager_unit_is_active(s->manager, s->scope)) -#endif // 0 return false; return true; @@ -1106,11 +1082,7 @@ SessionState session_get_state(Session *s) { if (s->stopping || s->timer_event_source) return SESSION_CLOSING; -#if 0 /// elogind does not support systemd scope_jobs if (s->scope_job || s->fifo_fd < 0) -#else - if (s->fifo_fd < 0) -#endif // 0 return SESSION_OPENING; if (session_is_active(s)) @@ -1122,27 +1094,10 @@ SessionState session_get_state(Session *s) { int session_kill(Session *s, KillWho who, int signo) { assert(s); -#if 0 /// Without direct cgroup support, elogind can not kill sessions if (!s->scope) return -ESRCH; return manager_kill_unit(s->manager, s->scope, who, signo, NULL); -#else - if (who == KILL_LEADER) { - if (s->leader <= 0) - return -ESRCH; - - /* FIXME: verify that leader is in cgroup? */ - - if (kill(s->leader, signo) < 0) { - return log_error_errno(errno, "Failed to kill process leader %d for session %s: %m", s->leader, s->id); - } - return 0; - } else - return cg_kill_recursive (SYSTEMD_CGROUP_CONTROLLER, s->id, signo, - CGROUP_IGNORE_SELF | CGROUP_REMOVE, - NULL, NULL, NULL); -#endif // 0 } static int session_open_vt(Session *s) { @@ -1360,6 +1315,7 @@ int session_set_controller(Session *s, const char *sender, bool force, bool prep session_release_controller(s, true); s->controller = name; name = NULL; + s->controller = TAKE_PTR(name); session_save(s); return 0; diff --git a/src/login/logind.c b/src/login/logind.c index 1251eb0ba..f431d31a8 100644 --- a/src/login/logind.c +++ b/src/login/logind.c @@ -41,6 +41,7 @@ #include "format-util.h" #include "fs-util.h" #include "logind.h" +//#include "parse-util.h" //#include "process-util.h" #include "selinux-util.h" #include "signal-util.h" @@ -438,6 +439,37 @@ static int manager_enumerate_users(Manager *m) { return r; } +static int parse_fdname(const char *fdname, char **session_id, dev_t *dev) { + _cleanup_strv_free_ char **parts = NULL; + _cleanup_free_ char *id = NULL; + unsigned int major, minor; + int r; + + parts = strv_split(fdname, "-"); + if (!parts) + return -ENOMEM; + if (strv_length(parts) != 5) + return -EINVAL; + + if (!streq(parts[0], "session")) + return -EINVAL; + id = strdup(parts[1]); + if (!id) + return -ENOMEM; + + if (!streq(parts[2], "device")) + return -EINVAL; + r = safe_atou(parts[3], &major) || + safe_atou(parts[4], &minor); + if (r < 0) + return r; + + *dev = makedev(major, minor); + *session_id = TAKE_PTR(id); + + return 0; +} + static int manager_attach_fds(Manager *m) { _cleanup_strv_free_ char **fdnames = NULL; int n, i, fd; @@ -451,16 +483,21 @@ static int manager_attach_fds(Manager *m) { return n; for (i = 0; i < n; i++) { + _cleanup_free_ char *id = NULL; + dev_t dev; struct stat st; SessionDevice *sd; Session *s; - char *id; + int r; fd = SD_LISTEN_FDS_START + i; - id = startswith(fdnames[i], "session-"); - if (!id) + r = parse_fdname(fdnames[i], &id, &dev); + if (r < 0) { + log_debug_errno(r, "Failed to parse fd name %s: %m", fdnames[i]); + close_nointr(fd); continue; + } s = hashmap_get(m->sessions, id); if (!s) { @@ -480,24 +517,24 @@ static int manager_attach_fds(Manager *m) { continue; } - if (!S_ISCHR(st.st_mode) && !S_ISBLK(st.st_mode)) { - log_debug("Device fd doesn't actually point to device node: %m"); + if (!S_ISCHR(st.st_mode) || st.st_rdev != dev) { + log_debug("Device fd doesn't point to the expected character device node"); close_nointr(fd); continue; } - sd = hashmap_get(s->devices, &st.st_rdev); + sd = hashmap_get(s->devices, &dev); if (!sd) { /* Weird, we got an fd for a session device which wasn't - * recorded in the session state file... */ + * recorded in the session state file... */ log_warning("Got fd for missing session device [%u:%u] in session %s", - major(st.st_rdev), minor(st.st_rdev), s->id); + major(dev), minor(dev), s->id); close_nointr(fd); continue; } log_debug("Attaching fd to session device [%u:%u] for session %s", - major(st.st_rdev), minor(st.st_rdev), s->id); + major(dev), minor(dev), s->id); session_device_attach_fd(sd, fd, s->was_active); } diff --git a/src/shared/bus-util.c b/src/shared/bus-util.c index ce0e2a31a..f0549f8d4 100644 --- a/src/shared/bus-util.c +++ b/src/shared/bus-util.c @@ -660,15 +660,15 @@ int bus_connect_user_systemd(sd_bus **_bus) { printf("%s=" fmt "\n", name, __VA_ARGS__); \ } while (0) -int bus_print_property(const char *name, sd_bus_message *property, bool value, bool all) { +int bus_print_property(const char *name, sd_bus_message *m, bool value, bool all) { char type; const char *contents; int r; assert(name); - assert(property); + assert(m); - r = sd_bus_message_peek_type(property, &type, &contents); + r = sd_bus_message_peek_type(m, &type, &contents); if (r < 0) return r; @@ -677,7 +677,7 @@ int bus_print_property(const char *name, sd_bus_message *property, bool value, b case SD_BUS_TYPE_STRING: { const char *s; - r = sd_bus_message_read_basic(property, type, &s); + r = sd_bus_message_read_basic(m, type, &s); if (r < 0) return r; @@ -696,7 +696,7 @@ int bus_print_property(const char *name, sd_bus_message *property, bool value, b case SD_BUS_TYPE_BOOLEAN: { int b; - r = sd_bus_message_read_basic(property, type, &b); + r = sd_bus_message_read_basic(m, type, &b); if (r < 0) return r; @@ -708,7 +708,7 @@ int bus_print_property(const char *name, sd_bus_message *property, bool value, b case SD_BUS_TYPE_UINT64: { uint64_t u; - r = sd_bus_message_read_basic(property, type, &u); + r = sd_bus_message_read_basic(m, type, &u); if (r < 0) return r; @@ -785,7 +785,7 @@ int bus_print_property(const char *name, sd_bus_message *property, bool value, b case SD_BUS_TYPE_INT64: { int64_t i; - r = sd_bus_message_read_basic(property, type, &i); + r = sd_bus_message_read_basic(m, type, &i); if (r < 0) return r; @@ -797,7 +797,7 @@ int bus_print_property(const char *name, sd_bus_message *property, bool value, b case SD_BUS_TYPE_UINT32: { uint32_t u; - r = sd_bus_message_read_basic(property, type, &u); + r = sd_bus_message_read_basic(m, type, &u); if (r < 0) return r; @@ -822,7 +822,7 @@ int bus_print_property(const char *name, sd_bus_message *property, bool value, b case SD_BUS_TYPE_INT32: { int32_t i; - r = sd_bus_message_read_basic(property, type, &i); + r = sd_bus_message_read_basic(m, type, &i); if (r < 0) return r; @@ -833,7 +833,7 @@ int bus_print_property(const char *name, sd_bus_message *property, bool value, b case SD_BUS_TYPE_DOUBLE: { double d; - r = sd_bus_message_read_basic(property, type, &d); + r = sd_bus_message_read_basic(m, type, &d); if (r < 0) return r; @@ -846,11 +846,11 @@ int bus_print_property(const char *name, sd_bus_message *property, bool value, b bool first = true; const char *str; - r = sd_bus_message_enter_container(property, SD_BUS_TYPE_ARRAY, contents); + r = sd_bus_message_enter_container(m, SD_BUS_TYPE_ARRAY, contents); if (r < 0) return r; - while ((r = sd_bus_message_read_basic(property, SD_BUS_TYPE_STRING, &str)) > 0) { + while ((r = sd_bus_message_read_basic(m, SD_BUS_TYPE_STRING, &str)) > 0) { bool good; if (first && !value) @@ -872,7 +872,7 @@ int bus_print_property(const char *name, sd_bus_message *property, bool value, b if (!first || all) puts(""); - r = sd_bus_message_exit_container(property); + r = sd_bus_message_exit_container(m); if (r < 0) return r; @@ -882,7 +882,7 @@ int bus_print_property(const char *name, sd_bus_message *property, bool value, b const uint8_t *u; size_t n; - r = sd_bus_message_read_array(property, SD_BUS_TYPE_BYTE, (const void**) &u, &n); + r = sd_bus_message_read_array(m, SD_BUS_TYPE_BYTE, (const void**) &u, &n); if (r < 0) return r; @@ -904,7 +904,7 @@ int bus_print_property(const char *name, sd_bus_message *property, bool value, b uint32_t *u; size_t n; - r = sd_bus_message_read_array(property, SD_BUS_TYPE_UINT32, (const void**) &u, &n); + r = sd_bus_message_read_array(m, SD_BUS_TYPE_UINT32, (const void**) &u, &n); if (r < 0) return r; @@ -929,81 +929,118 @@ int bus_print_property(const char *name, sd_bus_message *property, bool value, b return 0; } -int bus_print_all_properties(sd_bus *bus, const char *dest, const char *path, char **filter, bool value, bool all) { - _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; - _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; - int r; +int bus_message_print_all_properties( + sd_bus_message *m, + bus_message_print_t func, + char **filter, + bool value, + bool all, + Set **found_properties) { - assert(bus); - assert(path); + int r; - r = sd_bus_call_method(bus, - dest, - path, - "org.freedesktop.DBus.Properties", - "GetAll", - &error, - &reply, - "s", ""); - if (r < 0) - return r; + assert(m); - r = sd_bus_message_enter_container(reply, SD_BUS_TYPE_ARRAY, "{sv}"); + r = sd_bus_message_enter_container(m, SD_BUS_TYPE_ARRAY, "{sv}"); if (r < 0) return r; - while ((r = sd_bus_message_enter_container(reply, SD_BUS_TYPE_DICT_ENTRY, "sv")) > 0) { + while ((r = sd_bus_message_enter_container(m, SD_BUS_TYPE_DICT_ENTRY, "sv")) > 0) { const char *name; const char *contents; - r = sd_bus_message_read_basic(reply, SD_BUS_TYPE_STRING, &name); + r = sd_bus_message_read_basic(m, SD_BUS_TYPE_STRING, &name); if (r < 0) return r; + if (found_properties) { + r = set_ensure_allocated(found_properties, &string_hash_ops); + if (r < 0) + return log_oom(); + + r = set_put(*found_properties, name); + if (r < 0 && r != EEXIST) + return log_oom(); + } + if (!filter || strv_find(filter, name)) { - r = sd_bus_message_peek_type(reply, NULL, &contents); + r = sd_bus_message_peek_type(m, NULL, &contents); if (r < 0) return r; - r = sd_bus_message_enter_container(reply, SD_BUS_TYPE_VARIANT, contents); + r = sd_bus_message_enter_container(m, SD_BUS_TYPE_VARIANT, contents); if (r < 0) return r; - r = bus_print_property(name, reply, value, all); + if (func) + r = func(name, m, value, all); + if (!func || r == 0) + r = bus_print_property(name, m, value, all); if (r < 0) return r; if (r == 0) { if (all) printf("%s=[unprintable]\n", name); /* skip what we didn't read */ - r = sd_bus_message_skip(reply, contents); + r = sd_bus_message_skip(m, contents); if (r < 0) return r; } - r = sd_bus_message_exit_container(reply); + r = sd_bus_message_exit_container(m); if (r < 0) return r; } else { - r = sd_bus_message_skip(reply, "v"); + r = sd_bus_message_skip(m, "v"); if (r < 0) return r; } - r = sd_bus_message_exit_container(reply); + r = sd_bus_message_exit_container(m); if (r < 0) return r; } if (r < 0) return r; - r = sd_bus_message_exit_container(reply); + r = sd_bus_message_exit_container(m); if (r < 0) return r; return 0; } +int bus_print_all_properties( + sd_bus *bus, + const char *dest, + const char *path, + bus_message_print_t func, + char **filter, + bool value, + bool all, + Set **found_properties) { + + _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; + _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; + int r; + + assert(bus); + assert(path); + + r = sd_bus_call_method(bus, + dest, + path, + "org.freedesktop.DBus.Properties", + "GetAll", + &error, + &reply, + "s", ""); + if (r < 0) + return r; + + return bus_message_print_all_properties(reply, func, filter, value, all, found_properties); +} + int bus_map_id128(sd_bus *bus, const char *member, sd_bus_message *m, sd_bus_error *error, void *userdata) { sd_id128_t *p = userdata; const void *v; @@ -1024,7 +1061,7 @@ int bus_map_id128(sd_bus *bus, const char *member, sd_bus_message *m, sd_bus_err return 0; } -static int map_basic(sd_bus *bus, const char *member, sd_bus_message *m, sd_bus_error *error, void *userdata) { +static int map_basic(sd_bus *bus, const char *member, sd_bus_message *m, sd_bus_error *error, void *userdata, bool copy_string) { char type; int r; @@ -1035,7 +1072,7 @@ static int map_basic(sd_bus *bus, const char *member, sd_bus_message *m, sd_bus_ switch (type) { case SD_BUS_TYPE_STRING: { - char **p = userdata; + const char **p = userdata; const char *s; r = sd_bus_message_read_basic(m, type, &s); @@ -1045,7 +1082,11 @@ static int map_basic(sd_bus *bus, const char *member, sd_bus_message *m, sd_bus_ if (isempty(s)) s = NULL; - return free_and_strdup(p, s); + if (copy_string) + return free_and_strdup((char **) userdata, s); + + *p = s; + return 0; } case SD_BUS_TYPE_ARRAY: { @@ -1057,20 +1098,19 @@ static int map_basic(sd_bus *bus, const char *member, sd_bus_message *m, sd_bus_ return r; strv_free(*p); - *p = l; - l = NULL; + *p = TAKE_PTR(l); return 0; } case SD_BUS_TYPE_BOOLEAN: { unsigned b; - int *p = userdata; + bool *p = userdata; r = sd_bus_message_read_basic(m, type, &b); if (r < 0) return r; - *p = b; + *p = !!b; return 0; } @@ -1115,6 +1155,7 @@ static int map_basic(sd_bus *bus, const char *member, sd_bus_message *m, sd_bus_ int bus_message_map_all_properties( sd_bus_message *m, const struct bus_properties_map *map, + bool copy_string, sd_bus_error *error, void *userdata) { @@ -1157,7 +1198,7 @@ int bus_message_map_all_properties( if (map[i].set) r = prop->set(sd_bus_message_get_bus(m), member, m, error, v); else - r = map_basic(sd_bus_message_get_bus(m), member, m, error, v); + r = map_basic(sd_bus_message_get_bus(m), member, m, error, v, copy_string); if (r < 0) return r; @@ -1184,6 +1225,7 @@ int bus_message_map_all_properties( int bus_message_map_properties_changed( sd_bus_message *m, const struct bus_properties_map *map, + bool copy_string, sd_bus_error *error, void *userdata) { @@ -1193,7 +1235,7 @@ int bus_message_map_properties_changed( assert(m); assert(map); - r = bus_message_map_all_properties(m, map, error, userdata); + r = bus_message_map_all_properties(m, map, copy_string, error, userdata); if (r < 0) return r; @@ -1225,6 +1267,7 @@ int bus_map_all_properties( const char *path, const struct bus_properties_map *map, sd_bus_error *error, + sd_bus_message **reply, void *userdata) { _cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL; @@ -1247,7 +1290,14 @@ int bus_map_all_properties( if (r < 0) return r; - return bus_message_map_all_properties(m, map, error, userdata); + r = bus_message_map_all_properties(m, map, !reply, error, userdata); + if (r < 0) + return r; + + if (reply) + *reply = sd_bus_message_ref(m); + + return r; } int bus_connect_transport(BusTransport transport, const char *host, bool user, sd_bus **ret) { diff --git a/src/shared/nsflags.c b/src/shared/nsflags.c index 4879419f3..1a9c8ca07 100644 --- a/src/shared/nsflags.c +++ b/src/shared/nsflags.c @@ -116,8 +116,7 @@ int namespace_flag_to_string_many(unsigned long flags, char **ret) { return -ENOMEM; } - *ret = s; - s = NULL; + *ret = TAKE_PTR(s); return 0; } -- 2.30.2