From 385530342869ce4c6ea7388262bec4a0398b9311 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 7 Oct 2022 09:15:44 +0200 Subject: [PATCH] core,logind,systemctl,journald: replace calls to strerror() with setting errno + %m strerror() is not thread safe and calling it just isn't worth the effort required to justify why it would be safe in those cases. It's easier to just use %m which is thread-safe out of the box. I don't think that any of the changes in the patch cause any functional difference. This is just about getting rid of calls to strerror() in general. When we print an error message and fail to format the string, using something like "(null)" is good enough. This is very very unlikely to happen anyway. --- src/core/cgroup.c | 16 ++++++---------- src/core/execute.c | 3 ++- src/core/selinux-access.c | 22 ++++++++++------------ src/core/socket.c | 12 +++++------- src/core/unit-serialize.c | 6 ++++-- src/journal/journald-server.c | 3 ++- src/systemctl/systemctl-show.c | 6 ++++-- 7 files changed, 33 insertions(+), 35 deletions(-) diff --git a/src/core/cgroup.c b/src/core/cgroup.c index 6d40faa48b..bbd107849c 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -398,17 +398,13 @@ static char *format_cgroup_memory_limit_comparison(char *buf, size_t l, Unit *u, * In the absence of reliably being able to detect whether memcg swap support is available or not, * only complain if the error is not ENOENT. */ if (r > 0 || IN_SET(r, -ENODATA, -EOWNERDEAD) || - (r == -ENOENT && streq(property_name, "MemorySwapMax"))) { + (r == -ENOENT && streq(property_name, "MemorySwapMax"))) buf[0] = 0; - return buf; - } - - if (r < 0) { - (void) snprintf(buf, l, " (error getting kernel value: %s)", strerror_safe(r)); - return buf; - } - - (void) snprintf(buf, l, " (different value in kernel: %" PRIu64 ")", kval); + else if (r < 0) { + errno = -r; + (void) snprintf(buf, l, " (error getting kernel value: %m)"); + } else + (void) snprintf(buf, l, " (different value in kernel: %" PRIu64 ")", kval); return buf; } diff --git a/src/core/execute.c b/src/core/execute.c index 6a4e1e0954..a2ed66f3ca 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -6428,9 +6428,10 @@ static void exec_command_dump(ExecCommand *c, FILE *f, const char *prefix) { prefix2 = strjoina(prefix, "\t"); cmd = quote_command_line(c->argv, SHELL_ESCAPE_EMPTY); + fprintf(f, "%sCommand Line: %s\n", - prefix, cmd ?: strerror_safe(ENOMEM)); + prefix, strnull(cmd)); exec_status_dump(&c->exec_status, f, prefix2); } diff --git a/src/core/selinux-access.c b/src/core/selinux-access.c index 848ae246a7..c69baa8a1a 100644 --- a/src/core/selinux-access.c +++ b/src/core/selinux-access.c @@ -137,6 +137,7 @@ _printf_(2, 3) static int log_callback(int type, const char *fmt, ...) { } static int access_init(sd_bus_error *error) { + int r; if (!mac_selinux_use()) return 0; @@ -145,23 +146,20 @@ static int access_init(sd_bus_error *error) { return 1; if (avc_open(NULL, 0) != 0) { - int saved_errno = errno; - bool enforce; + r = -errno; /* Save original errno for later */ - enforce = security_getenforce() != 0; - log_full_errno(enforce ? LOG_ERR : LOG_WARNING, saved_errno, "Failed to open the SELinux AVC: %m"); + bool enforce = security_getenforce() != 0; + log_full_errno(enforce ? LOG_ERR : LOG_WARNING, r, "Failed to open the SELinux AVC: %m"); - /* If enforcement isn't on, then let's suppress this - * error, and just don't do any AVC checks. The - * warning we printed is hence all the admin will - * see. */ + /* If enforcement isn't on, then let's suppress this error, and just don't do any AVC checks. + * The warning we printed is hence all the admin will see. */ if (!enforce) return 0; - /* Return an access denied error, if we couldn't load - * the AVC but enforcing mode was on, or we couldn't - * determine whether it is one. */ - return sd_bus_error_setf(error, SD_BUS_ERROR_ACCESS_DENIED, "Failed to open the SELinux AVC: %s", strerror_safe(saved_errno)); + /* Return an access denied error based on the original errno, if we couldn't load the AVC but + * enforcing mode was on, or we couldn't determine whether it is one. */ + errno = -r; + return sd_bus_error_setf(error, SD_BUS_ERROR_ACCESS_DENIED, "Failed to open the SELinux AVC: %m"); } selinux_set_callback(SELINUX_CB_AUDIT, (union selinux_callback) { .func_audit = audit_callback }); diff --git a/src/core/socket.c b/src/core/socket.c index 308f84898c..2ac041f9d7 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -739,16 +739,14 @@ static void socket_dump(Unit *u, FILE *f, const char *prefix) { switch (p->type) { case SOCKET_SOCKET: { _cleanup_free_ char *k = NULL; - const char *t; int r; r = socket_address_print(&p->address, &k); - if (r < 0) - t = strerror_safe(r); - else - t = k; - - fprintf(f, "%s%s: %s\n", prefix, listen_lookup(socket_address_family(&p->address), p->address.type), t); + if (r < 0) { + errno = -r; + fprintf(f, "%s%s: %m\n", prefix, listen_lookup(socket_address_family(&p->address), p->address.type)); + } else + fprintf(f, "%s%s: %s\n", prefix, listen_lookup(socket_address_family(&p->address), p->address.type), k); break; } case SOCKET_SPECIAL: diff --git a/src/core/unit-serialize.c b/src/core/unit-serialize.c index 797bf6c5ce..7fe9f3f231 100644 --- a/src/core/unit-serialize.c +++ b/src/core/unit-serialize.c @@ -849,8 +849,10 @@ void unit_dump(Unit *u, FILE *f, const char *prefix) { fprintf(f, "%s\tMerged into: %s\n", prefix, u->merged_into->id); - else if (u->load_state == UNIT_ERROR) - fprintf(f, "%s\tLoad Error Code: %s\n", prefix, strerror_safe(u->load_error)); + else if (u->load_state == UNIT_ERROR) { + errno = abs(u->load_error); + fprintf(f, "%s\tLoad Error Code: %m\n", prefix); + } for (const char *n = sd_bus_track_first(u->bus_track); n; n = sd_bus_track_next(u->bus_track)) fprintf(f, "%s\tBus Ref: %s\n", prefix, n); diff --git a/src/journal/journald-server.c b/src/journal/journald-server.c index bfa9f44a83..71d7a59bda 100644 --- a/src/journal/journald-server.c +++ b/src/journal/journald-server.c @@ -1094,7 +1094,8 @@ void server_driver_message(Server *s, pid_t object_pid, const char *message_id, /* We failed to format the message. Emit a warning instead. */ char buf[LINE_MAX]; - xsprintf(buf, "MESSAGE=Entry printing failed: %s", strerror_safe(r)); + errno = -r; + xsprintf(buf, "MESSAGE=Entry printing failed: %m"); n = 3; iovec[n++] = IOVEC_MAKE_STRING("PRIORITY=4"); diff --git a/src/systemctl/systemctl-show.c b/src/systemctl/systemctl-show.c index 4c7094678c..8d3db98c0a 100644 --- a/src/systemctl/systemctl-show.c +++ b/src/systemctl/systemctl-show.c @@ -670,8 +670,10 @@ static void print_status_info( if (i->status_text) printf(" Status: \"%s\"\n", i->status_text); - if (i->status_errno > 0) - printf(" Error: %i (%s)\n", i->status_errno, strerror_safe(i->status_errno)); + if (i->status_errno > 0) { + errno = i->status_errno; + printf(" Error: %i (%m)\n", i->status_errno); + } if (i->ip_ingress_bytes != UINT64_MAX && i->ip_egress_bytes != UINT64_MAX) printf(" IP: %s in, %s out\n", -- 2.25.1