Compare commits

..

9 Commits

Author SHA1 Message Date
Yu Watanabe faa73d4e0c license: LGPL-2.1+ -> LGPL-2.1-or-later
Follow-up for db9ecf0501.
2020-11-12 10:50:23 +01:00
Yu Watanabe 9429ee6a89
Merge pull request #17567 from keszybz/various-small-cleanups
Various small cleanups
2020-11-12 16:30:06 +09:00
Zbigniew Jędrzejewski-Szmek 4d5f52e77e basic/fileio: constify struct timespec arguments 2020-11-10 15:52:32 +01:00
Zbigniew Jędrzejewski-Szmek b5c474f69b libsystemd-network: add comment explaining unusual memory access
Inspired by coverity CID#1435984. I'm confused by the union definion every time I look at it...
Let's at least add a comment to help future readers.
2020-11-10 15:52:32 +01:00
Zbigniew Jędrzejewski-Szmek 44ee03d111 tree-wide: unsetenv cannot fail
... when called with a valid environment variable name. This means that
any time we call it with a fixed string, it is guaranteed to return 0.
(Also when the variable is not present in the environment block.)
2020-11-10 15:52:32 +01:00
Zbigniew Jędrzejewski-Szmek 063f9f0da9 basic/env-util: add little helper to call setenv or unsetenv 2020-11-10 15:48:14 +01:00
Zbigniew Jędrzejewski-Szmek 55c540d39f sd-event: minor modernization
With this change the pattern used for epoll_ctl() is the same in all calls in
this file. Consistency FTW!
2020-11-10 14:19:20 +01:00
Zbigniew Jędrzejewski-Szmek ac9f2640cb sd-event: increase n_enabled_child_sources just once
Neither source_child_pidfd_register() nor event_make_signal_data() look at
n_enabled_child_sources.
2020-11-10 14:19:20 +01:00
Zbigniew Jędrzejewski-Szmek d2eafe61ca sd-event: update state at the end in event_source_enable
Coverity in CID#1435966 was complaining that s->enabled is not "restored" in
all cases. But the code was actually correct, since it should only be
"restored" in the error paths. But let's still make this prettier by not setting
the state before all operations that may fail are done.

We need to set .enabled for the prioq reshuffling operations, so move those down.

No functional change intended.
2020-11-10 14:18:47 +01:00
21 changed files with 118 additions and 137 deletions

View File

@ -747,3 +747,15 @@ int getenv_bool_secure(const char *p) {
return parse_boolean(e);
}
int set_unset_env(const char *name, const char *value, bool overwrite) {
int r;
if (value)
r = setenv(name, value, overwrite);
else
r = unsetenv(name);
if (r < 0)
return -errno;
return 0;
}

View File

@ -52,3 +52,6 @@ char *strv_env_get(char **x, const char *n) _pure_;
int getenv_bool(const char *p);
int getenv_bool_secure(const char *p);
/* Like setenv, but calls unsetenv if value == NULL. */
int set_unset_env(const char *name, const char *value, bool overwrite);

View File

@ -117,7 +117,7 @@ int write_string_stream_ts(
FILE *f,
const char *line,
WriteStringFileFlags flags,
struct timespec *ts) {
const struct timespec *ts) {
bool needs_nl;
int r, fd;
@ -161,7 +161,7 @@ int write_string_stream_ts(
return r;
if (ts) {
struct timespec twice[2] = {*ts, *ts};
const struct timespec twice[2] = {*ts, *ts};
if (futimens(fd, twice) < 0)
return -errno;
@ -174,7 +174,7 @@ static int write_string_file_atomic(
const char *fn,
const char *line,
WriteStringFileFlags flags,
struct timespec *ts) {
const struct timespec *ts) {
_cleanup_fclose_ FILE *f = NULL;
_cleanup_free_ char *p = NULL;
@ -221,7 +221,7 @@ int write_string_file_ts(
const char *fn,
const char *line,
WriteStringFileFlags flags,
struct timespec *ts) {
const struct timespec *ts) {
_cleanup_fclose_ FILE *f = NULL;
int q, r, fd;

View File

@ -48,11 +48,11 @@ DIR* take_fdopendir(int *dfd);
FILE* open_memstream_unlocked(char **ptr, size_t *sizeloc);
FILE* fmemopen_unlocked(void *buf, size_t size, const char *mode);
int write_string_stream_ts(FILE *f, const char *line, WriteStringFileFlags flags, struct timespec *ts);
int write_string_stream_ts(FILE *f, const char *line, WriteStringFileFlags flags, const struct timespec *ts);
static inline int write_string_stream(FILE *f, const char *line, WriteStringFileFlags flags) {
return write_string_stream_ts(f, line, flags, NULL);
}
int write_string_file_ts(const char *fn, const char *line, WriteStringFileFlags flags, struct timespec *ts);
int write_string_file_ts(const char *fn, const char *line, WriteStringFileFlags flags, const struct timespec *ts);
static inline int write_string_file(const char *fn, const char *line, WriteStringFileFlags flags) {
return write_string_file_ts(fn, line, flags, NULL);
}

View File

@ -1418,9 +1418,8 @@ static int fixup_environment(void) {
return -errno;
/* The kernels sets HOME=/ for init. Let's undo this. */
if (path_equal_ptr(getenv("HOME"), "/") &&
unsetenv("HOME") < 0)
log_warning_errno(errno, "Failed to unset $HOME: %m");
if (path_equal_ptr(getenv("HOME"), "/"))
assert_se(unsetenv("HOME") == 0);
return 0;
}

View File

@ -215,9 +215,7 @@ static int acquire_existing_password(const char *user_name, UserRecord *hr, bool
return log_error_errno(r, "Failed to store password: %m");
string_erase(e);
if (unsetenv("PASSWORD") < 0)
return log_error_errno(errno, "Failed to unset $PASSWORD: %m");
assert_se(unsetenv("PASSWORD") == 0);
return 0;
}
@ -255,9 +253,7 @@ static int acquire_token_pin(const char *user_name, UserRecord *hr) {
return log_error_errno(r, "Failed to store token PIN: %m");
string_erase(e);
if (unsetenv("PIN") < 0)
return log_error_errno(errno, "Failed to unset $PIN: %m");
assert_se(unsetenv("PIN") == 0);
return 0;
}
@ -997,9 +993,7 @@ static int acquire_new_password(
return log_error_errno(r, "Failed to store password: %m");
string_erase(e);
if (unsetenv("NEWPASSWORD") < 0)
return log_error_errno(errno, "Failed to unset $NEWPASSWORD: %m");
assert_se(unsetenv("NEWPASSWORD") == 0);
if (ret)
*ret = TAKE_PTR(copy);

View File

@ -106,7 +106,7 @@ static int _bind_raw_socket(int ifindex, union sockaddr_union *link,
.sll_hatype = htobe16(arp_type),
.sll_halen = bcast_addr_len,
};
memcpy(link->ll.sll_addr, bcast_addr, bcast_addr_len);
memcpy(link->ll.sll_addr, bcast_addr, bcast_addr_len); /* We may overflow link->ll. link->ll_buffer ensures we have enough space. */
r = bind(s, &link->sa, SOCKADDR_LL_LEN(link->ll));
if (r < 0)

View File

@ -30,13 +30,12 @@
#define SNDBUF_SIZE (8*1024*1024)
static void unsetenv_all(bool unset_environment) {
if (!unset_environment)
return;
unsetenv("LISTEN_PID");
unsetenv("LISTEN_FDS");
unsetenv("LISTEN_FDNAMES");
assert_se(unsetenv("LISTEN_PID") == 0);
assert_se(unsetenv("LISTEN_FDS") == 0);
assert_se(unsetenv("LISTEN_FDNAMES") == 0);
}
_public_ int sd_listen_fds(int unset_environment) {
@ -548,7 +547,7 @@ _public_ int sd_pid_notify_with_fds(
finish:
if (unset_environment)
unsetenv("NOTIFY_SOCKET");
assert_se(unsetenv("NOTIFY_SOCKET") == 0);
return r;
}
@ -672,9 +671,9 @@ _public_ int sd_watchdog_enabled(int unset_environment, uint64_t *usec) {
finish:
if (unset_environment && s)
unsetenv("WATCHDOG_USEC");
assert_se(unsetenv("WATCHDOG_USEC") == 0);
if (unset_environment && p)
unsetenv("WATCHDOG_PID");
assert_se(unsetenv("WATCHDOG_PID") == 0);
return r;
}

View File

@ -402,8 +402,7 @@ static int source_io_register(
if (epoll_ctl(s->event->epoll_fd,
s->io.registered ? EPOLL_CTL_MOD : EPOLL_CTL_ADD,
s->io.fd,
&ev) < 0)
s->io.fd, &ev) < 0)
return -errno;
s->io.registered = true;
@ -430,8 +429,6 @@ static void source_child_pidfd_unregister(sd_event_source *s) {
}
static int source_child_pidfd_register(sd_event_source *s, int enabled) {
int r;
assert(s);
assert(s->type == SOURCE_CHILD);
assert(enabled != SD_EVENT_OFF);
@ -442,11 +439,9 @@ static int source_child_pidfd_register(sd_event_source *s, int enabled) {
.data.ptr = s,
};
if (s->child.registered)
r = epoll_ctl(s->event->epoll_fd, EPOLL_CTL_MOD, s->child.pidfd, &ev);
else
r = epoll_ctl(s->event->epoll_fd, EPOLL_CTL_ADD, s->child.pidfd, &ev);
if (r < 0)
if (epoll_ctl(s->event->epoll_fd,
s->child.registered ? EPOLL_CTL_MOD : EPOLL_CTL_ADD,
s->child.pidfd, &ev) < 0)
return -errno;
}
@ -1340,31 +1335,25 @@ _public_ int sd_event_add_child(
if (r < 0)
return r;
e->n_enabled_child_sources++;
if (EVENT_SOURCE_WATCH_PIDFD(s)) {
/* We have a pidfd and we only want to watch for exit */
r = source_child_pidfd_register(s, s->enabled);
if (r < 0) {
e->n_enabled_child_sources--;
if (r < 0)
return r;
}
} else {
/* We have no pidfd or we shall wait for some other event than WEXITED */
r = event_make_signal_data(e, SIGCHLD, NULL);
if (r < 0) {
e->n_enabled_child_sources--;
if (r < 0)
return r;
}
e->need_process_child = true;
}
e->n_enabled_child_sources++;
if (ret)
*ret = s;
TAKE_PTR(s);
return 0;
}
@ -1429,31 +1418,24 @@ _public_ int sd_event_add_child_pidfd(
if (r < 0)
return r;
e->n_enabled_child_sources++;
if (EVENT_SOURCE_WATCH_PIDFD(s)) {
/* We only want to watch for WEXITED */
r = source_child_pidfd_register(s, s->enabled);
if (r < 0) {
e->n_enabled_child_sources--;
if (r < 0)
return r;
}
} else {
/* We shall wait for some other event than WEXITED */
r = event_make_signal_data(e, SIGCHLD, NULL);
if (r < 0) {
e->n_enabled_child_sources--;
if (r < 0)
return r;
}
e->need_process_child = true;
}
e->n_enabled_child_sources++;
if (ret)
*ret = s;
TAKE_PTR(s);
return 0;
}
@ -2311,11 +2293,11 @@ static int event_source_disable(sd_event_source *s) {
return 0;
}
static int event_source_enable(sd_event_source *s, int m) {
static int event_source_enable(sd_event_source *s, int enable) {
int r;
assert(s);
assert(IN_SET(m, SD_EVENT_ON, SD_EVENT_ONESHOT));
assert(IN_SET(enable, SD_EVENT_ON, SD_EVENT_ONESHOT));
assert(s->enabled == SD_EVENT_OFF);
/* Unset the pending flag when this event source is enabled */
@ -2325,67 +2307,49 @@ static int event_source_enable(sd_event_source *s, int m) {
return r;
}
s->enabled = m;
switch (s->type) {
case SOURCE_IO:
r = source_io_register(s, m, s->io.events);
r = source_io_register(s, enable, s->io.events);
if (r < 0)
return r;
break;
case SOURCE_SIGNAL:
r = event_make_signal_data(s->event, s->signal.sig, NULL);
if (r < 0) {
s->enabled = SD_EVENT_OFF;
event_gc_signal_data(s->event, &s->priority, s->signal.sig);
return r;
}
break;
case SOURCE_CHILD:
if (EVENT_SOURCE_WATCH_PIDFD(s)) {
/* yes, we have pidfd */
r = source_child_pidfd_register(s, enable);
if (r < 0)
return r;
} else {
/* no pidfd, or something other to watch for than WEXITED */
r = event_make_signal_data(s->event, SIGCHLD, NULL);
if (r < 0) {
event_gc_signal_data(s->event, &s->priority, SIGCHLD);
return r;
}
}
s->event->n_enabled_child_sources++;
break;
case SOURCE_TIME_REALTIME:
case SOURCE_TIME_BOOTTIME:
case SOURCE_TIME_MONOTONIC:
case SOURCE_TIME_REALTIME_ALARM:
case SOURCE_TIME_BOOTTIME_ALARM:
event_source_time_prioq_reshuffle(s);
break;
case SOURCE_SIGNAL:
r = event_make_signal_data(s->event, s->signal.sig, NULL);
if (r < 0) {
s->enabled = SD_EVENT_OFF;
event_gc_signal_data(s->event, &s->priority, s->signal.sig);
return r;
}
break;
case SOURCE_CHILD:
s->event->n_enabled_child_sources++;
if (EVENT_SOURCE_WATCH_PIDFD(s)) {
/* yes, we have pidfd */
r = source_child_pidfd_register(s, s->enabled);
if (r < 0) {
s->enabled = SD_EVENT_OFF;
s->event->n_enabled_child_sources--;
return r;
}
} else {
/* no pidfd, or something other to watch for than WEXITED */
r = event_make_signal_data(s->event, SIGCHLD, NULL);
if (r < 0) {
s->enabled = SD_EVENT_OFF;
s->event->n_enabled_child_sources--;
event_gc_signal_data(s->event, &s->priority, SIGCHLD);
return r;
}
}
break;
case SOURCE_EXIT:
prioq_reshuffle(s->event->exit, s, &s->exit.prioq_index);
break;
case SOURCE_DEFER:
case SOURCE_POST:
case SOURCE_INOTIFY:
@ -2395,6 +2359,26 @@ static int event_source_enable(sd_event_source *s, int m) {
assert_not_reached("Wut? I shouldn't exist.");
}
s->enabled = enable;
/* Non-failing operations below */
switch (s->type) {
case SOURCE_TIME_REALTIME:
case SOURCE_TIME_BOOTTIME:
case SOURCE_TIME_MONOTONIC:
case SOURCE_TIME_REALTIME_ALARM:
case SOURCE_TIME_BOOTTIME_ALARM:
event_source_time_prioq_reshuffle(s);
break;
case SOURCE_EXIT:
prioq_reshuffle(s->event->exit, s, &s->exit.prioq_index);
break;
default:
break;
}
return 0;
}

View File

@ -189,12 +189,9 @@ int pager_open(PagerFlags flags) {
/* We generally always set variables used by less, even if we end up using a different pager.
* They shouldn't hurt in any case, and ideally other pagers would look at them too. */
if (use_secure_mode)
r = setenv("LESSSECURE", "1", 1);
else
r = unsetenv("LESSSECURE");
r = set_unset_env("LESSSECURE", use_secure_mode ? "1" : NULL, true);
if (r < 0) {
log_error_errno(errno, "Failed to adjust environment variable LESSSECURE: %m");
log_error_errno(r, "Failed to adjust environment variable LESSSECURE: %m");
_exit(EXIT_FAILURE);
}

View File

@ -372,10 +372,7 @@ static void test_environment_gathering(void) {
assert_se(streq(strv_env_get(env, "PATH"), DEFAULT_PATH ":/no/such/file"));
/* reset environ PATH */
if (old)
(void) setenv("PATH", old, 1);
else
(void) unsetenv("PATH");
assert_se(set_unset_env("PATH", old, true) == 0);
}
static void test_error_catching(void) {

View File

@ -898,11 +898,11 @@ int main(int argc, char *argv[]) {
}
#endif
(void) unsetenv("USER");
(void) unsetenv("LOGNAME");
(void) unsetenv("SHELL");
(void) unsetenv("HOME");
(void) unsetenv("TMPDIR");
assert_se(unsetenv("USER") == 0);
assert_se(unsetenv("LOGNAME") == 0);
assert_se(unsetenv("SHELL") == 0);
assert_se(unsetenv("HOME") == 0);
assert_se(unsetenv("TMPDIR") == 0);
can_unshare = have_namespaces();

View File

@ -184,7 +184,7 @@ static void test_find_executable_full(void) {
if (p)
assert_se(oldpath = strdup(p));
assert_se(unsetenv("PATH") >= 0);
assert_se(unsetenv("PATH") == 0);
assert_se(find_executable_full("sh", true, &p) == 0);
puts(p);
@ -347,7 +347,7 @@ static void test_fsck_exists(void) {
log_info("/* %s */", __func__);
/* Ensure we use a sane default for PATH. */
unsetenv("PATH");
assert_se(unsetenv("PATH") == 0);
/* fsck.minix is provided by util-linux and will probably exist. */
assert_se(fsck_exists("minix") == 1);

View File

@ -480,7 +480,7 @@ static void test_in_utc_timezone(void) {
assert_se(streq(tzname[0], "CET"));
assert_se(streq(tzname[1], "CEST"));
assert_se(unsetenv("TZ") >= 0);
assert_se(unsetenv("TZ") == 0);
}
static void test_map_clock_usec(void) {

View File

@ -12,6 +12,7 @@
#include "bus-locator.h"
#include "bus-map-properties.h"
#include "bus-print-properties.h"
#include "env-util.h"
#include "format-table.h"
#include "in-addr-util.h"
#include "main-func.h"
@ -139,12 +140,9 @@ static int print_status_info(const StatusInfo *i) {
/* Restore the $TZ */
if (old_tz)
r = setenv("TZ", old_tz, true);
else
r = unsetenv("TZ");
r = set_unset_env("TZ", old_tz, true);
if (r < 0)
log_warning_errno(errno, "Failed to set TZ environment variable, ignoring: %m");
log_warning_errno(r, "Failed to set TZ environment variable, ignoring: %m");
else
tzset();

View File

@ -565,7 +565,7 @@ static int worker_main(Manager *_manager, sd_device_monitor *monitor, sd_device
assert(monitor);
assert(dev);
unsetenv("NOTIFY_SOCKET");
assert_se(unsetenv("NOTIFY_SOCKET") == 0);
assert_se(sigprocmask_many(SIG_BLOCK, NULL, SIGTERM, -1) >= 0);

View File

@ -780,10 +780,8 @@ static int run(int argc, char *argv[]) {
return log_error_errno(r, "Failed to set $SYSTEMD_ONLY_USERDB: %m");
log_info("Enabled services: %s", e);
} else {
if (unsetenv("SYSTEMD_ONLY_USERDB") < 0)
return log_error_errno(r, "Failed to unset $SYSTEMD_ONLY_USERDB: %m");
}
} else
assert_se(unsetenv("SYSTEMD_ONLY_USERDB") == 0);
return dispatch_verb(argc, argv, verbs, NULL);
}

View File

@ -1,5 +1,5 @@
#!/usr/bin/env python3
# SPDX-License-Identifier: LGPL-2.1+
# SPDX-License-Identifier: LGPL-2.1-or-later
#
# sd-script.py: create LOTS of sd device entries in fake sysfs
#

View File

@ -1,4 +1,4 @@
# SPDX-License-Identifier: LGPL-2.1+
# SPDX-License-Identifier: LGPL-2.1-or-later
#
# This file is part of systemd.
#

View File

@ -1,4 +1,4 @@
# SPDX-License-Identifier: LGPL-2.1+
# SPDX-License-Identifier: LGPL-2.1-or-later
#
# This file is part of systemd.
#

View File

@ -1,4 +1,4 @@
# SPDX-License-Identifier: LGPL-2.1+
# SPDX-License-Identifier: LGPL-2.1-or-later
#
# This file is part of systemd.
#