From ee3f331e539dc37e88e558583c8a9a7a2383f5e0 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 6 Nov 2016 12:48:23 -0500 Subject: [PATCH] tree-wide: add SD_ID128_MAKE_STR, remove LOG_MESSAGE_ID Embedding sd_id128_t's in constant strings was rather cumbersome. We had SD_ID128_CONST_STR which returned a const char[], but it had two problems: - it wasn't possible to statically concatanate this array with a normal string - gcc wasn't really able to optimize this, and generated code to perform the "conversion" at runtime. Because of this, even our own code in coredumpctl wasn't using SD_ID128_CONST_STR. Add a new macro to generate a constant string: SD_ID128_MAKE_STR. It is not as elegant as SD_ID128_CONST_STR, because it requires a repetition of the numbers, but in practice it is more convenient to use, and allows gcc to generate smarter code: $ size .libs/elogind{,-logind,-journald}{.old,} text data bss dec hex filename 1265204 149564 4808 1419576 15a938 .libs/elogind.old 1260268 149564 4808 1414640 1595f0 .libs/elogind 246805 13852 209 260866 3fb02 .libs/elogind-logind.old 240973 13852 209 255034 3e43a .libs/elogind-logind 146839 4984 34 151857 25131 .libs/elogind-journald.old 146391 4984 34 151409 24f71 .libs/elogind-journald It is also much easier to check if a certain binary uses a certain MESSAGE_ID: $ strings .libs/elogind.old|grep MESSAGE_ID MESSAGE_ID=%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x MESSAGE_ID=%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x MESSAGE_ID=%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x MESSAGE_ID=%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x $ strings .libs/elogind|grep MESSAGE_ID MESSAGE_ID=c7a787079b354eaaa9e77b371893cd27 MESSAGE_ID=b07a249cd024414a82dd00cd181378ff MESSAGE_ID=641257651c1b4ec9a8624d7a40a9e1e7 MESSAGE_ID=de5b426a63be47a7b6ac3eaac82e2f6f MESSAGE_ID=d34d037fff1847e6ae669a370e694725 MESSAGE_ID=7d4958e842da4a758f6c1cdc7b36dcc5 MESSAGE_ID=1dee0369c7fc4736b7099b38ecb46ee7 MESSAGE_ID=39f53479d3a045ac8e11786248231fbf MESSAGE_ID=be02cf6855d2428ba40df7e9d022f03d MESSAGE_ID=7b05ebc668384222baa8881179cfda54 MESSAGE_ID=9d1aaa27d60140bd96365438aad20286 --- src/basic/log.c | 114 ++++++++++++++++++------------------- src/basic/log.h | 3 +- src/login/logind-button.c | 14 ++--- src/login/logind-dbus.c | 17 ++---- src/login/logind-seat.c | 8 +-- src/login/logind-session.c | 15 ++--- src/sleep/sleep.c | 4 +- 7 files changed, 80 insertions(+), 95 deletions(-) diff --git a/src/basic/log.c b/src/basic/log.c index d84c9bf11..0783372c5 100644 --- a/src/basic/log.c +++ b/src/basic/log.c @@ -37,7 +37,7 @@ #include "alloc-util.h" #include "fd-util.h" -#include "formats-util.h" +#include "format-util.h" #include "io-util.h" #include "log.h" #include "macro.h" @@ -249,14 +249,14 @@ int log_open(void) { return 0; } - if ((log_target != LOG_TARGET_AUTO && log_target != LOG_TARGET_SAFE) || + if (!IN_SET(log_target, LOG_TARGET_AUTO, LOG_TARGET_SAFE) || getpid() == 1 || isatty(STDERR_FILENO) <= 0) { #if 0 /// elogind does not support logging to systemd-journald - if (log_target == LOG_TARGET_AUTO || - log_target == LOG_TARGET_JOURNAL_OR_KMSG || - log_target == LOG_TARGET_JOURNAL) { + if (IN_SET(log_target, LOG_TARGET_AUTO, + LOG_TARGET_JOURNAL_OR_KMSG, + LOG_TARGET_JOURNAL)) { r = log_open_journal(); if (r >= 0) { log_close_syslog(); @@ -266,8 +266,8 @@ int log_open(void) { } #endif // 0 - if (log_target == LOG_TARGET_SYSLOG_OR_KMSG || - log_target == LOG_TARGET_SYSLOG) { + if (IN_SET(log_target, LOG_TARGET_SYSLOG_OR_KMSG, + LOG_TARGET_SYSLOG)) { r = log_open_syslog(); if (r >= 0) { log_close_journal(); @@ -276,11 +276,11 @@ int log_open(void) { } } - if (log_target == LOG_TARGET_AUTO || - log_target == LOG_TARGET_SAFE || - log_target == LOG_TARGET_JOURNAL_OR_KMSG || - log_target == LOG_TARGET_SYSLOG_OR_KMSG || - log_target == LOG_TARGET_KMSG) { + if (IN_SET(log_target, LOG_TARGET_AUTO, + LOG_TARGET_SAFE, + LOG_TARGET_JOURNAL_OR_KMSG, + LOG_TARGET_SYSLOG_OR_KMSG, + LOG_TARGET_KMSG)) { r = log_open_kmsg(); if (r >= 0) { log_close_journal(); @@ -513,7 +513,7 @@ static int log_do_header( line ? "CODE_LINE=" : "", line ? 1 : 0, line, /* %.0d means no output too, special case for 0 */ line ? "\n" : "", - isempty(func) ? "" : "CODE_FUNCTION=", + isempty(func) ? "" : "CODE_FUNC=", isempty(func) ? "" : func, isempty(func) ? "" : "\n", error ? "ERRNO=" : "", @@ -603,9 +603,9 @@ static int log_dispatch( *(e++) = 0; #if 0 /// elogind does not support logging to systemd-journald - if (log_target == LOG_TARGET_AUTO || - log_target == LOG_TARGET_JOURNAL_OR_KMSG || - log_target == LOG_TARGET_JOURNAL) { + if (IN_SET(log_target, LOG_TARGET_AUTO, + LOG_TARGET_JOURNAL_OR_KMSG, + LOG_TARGET_JOURNAL)) { k = write_to_journal(level, error, file, line, func, object_field, object, extra_field, extra, buffer); if (k < 0) { @@ -616,8 +616,8 @@ static int log_dispatch( } #endif // 0 - if (log_target == LOG_TARGET_SYSLOG_OR_KMSG || - log_target == LOG_TARGET_SYSLOG) { + if (IN_SET(log_target, LOG_TARGET_SYSLOG_OR_KMSG, + LOG_TARGET_SYSLOG)) { k = write_to_syslog(level, error, file, line, func, buffer); if (k < 0) { @@ -628,11 +628,11 @@ static int log_dispatch( } if (k <= 0 && - (log_target == LOG_TARGET_AUTO || - log_target == LOG_TARGET_SAFE || - log_target == LOG_TARGET_SYSLOG_OR_KMSG || - log_target == LOG_TARGET_JOURNAL_OR_KMSG || - log_target == LOG_TARGET_KMSG)) { + IN_SET(log_target, LOG_TARGET_AUTO, + LOG_TARGET_SAFE, + LOG_TARGET_SYSLOG_OR_KMSG, + LOG_TARGET_JOURNAL_OR_KMSG, + LOG_TARGET_KMSG)) { k = write_to_kmsg(level, error, file, line, func, buffer); if (k < 0) { @@ -898,9 +898,9 @@ int log_struct_internal( level = log_facility | LOG_PRI(level); #if 0 /// elogind does not support logging to systemd-journald - if ((log_target == LOG_TARGET_AUTO || - log_target == LOG_TARGET_JOURNAL_OR_KMSG || - log_target == LOG_TARGET_JOURNAL) && + if (IN_SET(log_target, LOG_TARGET_AUTO, + LOG_TARGET_JOURNAL_OR_KMSG, + LOG_TARGET_JOURNAL) && journal_fd >= 0) { char header[LINE_MAX]; struct iovec iovec[17] = {}; @@ -999,24 +999,30 @@ static int parse_proc_cmdline_item(const char *key, const char *value, void *dat if (streq(key, "debug") && !value) log_set_max_level(LOG_DEBUG); - else if (streq(key, "systemd.log_target") && value) { + else if (proc_cmdline_key_streq(key, "systemd.log_target")) { + + if (proc_cmdline_value_missing(key, value)) + return 0; if (log_set_target_from_string(value) < 0) log_warning("Failed to parse log target '%s'. Ignoring.", value); - } else if (streq(key, "systemd.log_level") && value) { + } else if (proc_cmdline_key_streq(key, "systemd.log_level")) { + + if (proc_cmdline_value_missing(key, value)) + return 0; if (log_set_max_level_from_string(value) < 0) log_warning("Failed to parse log level '%s'. Ignoring.", value); - } else if (streq(key, "systemd.log_color") && value) { + } else if (proc_cmdline_key_streq(key, "systemd.log_color")) { - if (log_show_color_from_string(value) < 0) + if (log_show_color_from_string(value ?: "1") < 0) log_warning("Failed to parse log color setting '%s'. Ignoring.", value); - } else if (streq(key, "systemd.log_location") && value) { + } else if (proc_cmdline_key_streq(key, "systemd.log_location")) { - if (log_show_location_from_string(value) < 0) + if (log_show_location_from_string(value ?: "1") < 0) log_warning("Failed to parse log location setting '%s'. Ignoring.", value); } @@ -1027,10 +1033,9 @@ void log_parse_environment(void) { const char *e; if (get_ctty_devnr(0, NULL) < 0) - /* Only try to read the command line in daemons. - We assume that anything that has a controlling - tty is user stuff. */ - (void) parse_proc_cmdline(parse_proc_cmdline_item, NULL, true); + /* Only try to read the command line in daemons. We assume that anything that has a controlling tty is + user stuff. */ + (void) proc_cmdline_parse(parse_proc_cmdline_item, NULL, PROC_CMDLINE_STRIP_RD_PREFIX); e = secure_getenv("SYSTEMD_LOG_TARGET"); if (e && log_set_target_from_string(e) < 0) @@ -1096,8 +1101,8 @@ int log_show_location_from_string(const char *e) { } bool log_on_console(void) { - if (log_target == LOG_TARGET_CONSOLE || - log_target == LOG_TARGET_CONSOLE_PREFIXED) + if (IN_SET(log_target, LOG_TARGET_CONSOLE, + LOG_TARGET_CONSOLE_PREFIXED)) return true; return syslog_fd < 0 && kmsg_fd < 0 && journal_fd < 0; @@ -1156,8 +1161,8 @@ int log_syntax_internal( PROTECT_ERRNO; char buffer[LINE_MAX]; - int r; va_list ap; + const char *unit_fmt = NULL; if (error < 0) error = -error; @@ -1176,24 +1181,15 @@ int log_syntax_internal( va_end(ap); if (unit) - r = log_struct_internal( - level, error, - file, line, func, - getpid() == 1 ? "UNIT=%s" : "USER_UNIT=%s", unit, - LOG_MESSAGE_ID(SD_MESSAGE_INVALID_CONFIGURATION), - "CONFIG_FILE=%s", config_file, - "CONFIG_LINE=%u", config_line, - LOG_MESSAGE("[%s:%u] %s", config_file, config_line, buffer), - NULL); - else - r = log_struct_internal( - level, error, - file, line, func, - LOG_MESSAGE_ID(SD_MESSAGE_INVALID_CONFIGURATION), - "CONFIG_FILE=%s", config_file, - "CONFIG_LINE=%u", config_line, - LOG_MESSAGE("[%s:%u] %s", config_file, config_line, buffer), - NULL); - - return r; + unit_fmt = getpid() == 1 ? "UNIT=%s" : "USER_UNIT=%s"; + + return log_struct_internal( + level, error, + file, line, func, + "MESSAGE_ID=" SD_MESSAGE_INVALID_CONFIGURATION_STR, + "CONFIG_FILE=%s", config_file, + "CONFIG_LINE=%u", config_line, + LOG_MESSAGE("%s:%u: %s", config_file, config_line, buffer), + unit_fmt, unit, + NULL); } diff --git a/src/basic/log.h b/src/basic/log.h index 2e1d8c583..9505192c7 100644 --- a/src/basic/log.h +++ b/src/basic/log.h @@ -221,9 +221,8 @@ bool log_on_console(void) _pure_; const char *log_target_to_string(LogTarget target) _const_; LogTarget log_target_from_string(const char *s) _pure_; -/* Helpers to prepare various fields for structured logging */ +/* Helper to prepare various field for structured logging */ #define LOG_MESSAGE(fmt, ...) "MESSAGE=" fmt, ##__VA_ARGS__ -#define LOG_MESSAGE_ID(x) "MESSAGE_ID=" SD_ID128_FORMAT_STR, SD_ID128_FORMAT_VAL(x) #if 0 /// UNNEEDED by elogind void log_received_signal(int level, const struct signalfd_siginfo *si); diff --git a/src/login/logind-button.c b/src/login/logind-button.c index 90fb93bba..d739af8ea 100644 --- a/src/login/logind-button.c +++ b/src/login/logind-button.c @@ -155,7 +155,7 @@ static int button_dispatch(sd_event_source *s, int fd, uint32_t revents, void *u case KEY_POWER2: log_struct(LOG_INFO, LOG_MESSAGE("Power key pressed."), - LOG_MESSAGE_ID(SD_MESSAGE_POWER_KEY), + "MESSAGE_ID=" SD_MESSAGE_POWER_KEY_STR, NULL); manager_handle_action(b->manager, INHIBIT_HANDLE_POWER_KEY, b->manager->handle_power_key, b->manager->power_key_ignore_inhibited, true); @@ -170,7 +170,7 @@ static int button_dispatch(sd_event_source *s, int fd, uint32_t revents, void *u case KEY_SLEEP: log_struct(LOG_INFO, LOG_MESSAGE("Suspend key pressed."), - LOG_MESSAGE_ID(SD_MESSAGE_SUSPEND_KEY), + "MESSAGE_ID=" SD_MESSAGE_SUSPEND_KEY_STR, NULL); manager_handle_action(b->manager, INHIBIT_HANDLE_SUSPEND_KEY, b->manager->handle_suspend_key, b->manager->suspend_key_ignore_inhibited, true); @@ -179,7 +179,7 @@ static int button_dispatch(sd_event_source *s, int fd, uint32_t revents, void *u case KEY_SUSPEND: log_struct(LOG_INFO, LOG_MESSAGE("Hibernate key pressed."), - LOG_MESSAGE_ID(SD_MESSAGE_HIBERNATE_KEY), + "MESSAGE_ID=" SD_MESSAGE_HIBERNATE_KEY_STR, NULL); manager_handle_action(b->manager, INHIBIT_HANDLE_HIBERNATE_KEY, b->manager->handle_hibernate_key, b->manager->hibernate_key_ignore_inhibited, true); @@ -191,7 +191,7 @@ static int button_dispatch(sd_event_source *s, int fd, uint32_t revents, void *u if (ev.code == SW_LID) { log_struct(LOG_INFO, LOG_MESSAGE("Lid closed."), - LOG_MESSAGE_ID(SD_MESSAGE_LID_CLOSED), + "MESSAGE_ID=" SD_MESSAGE_LID_CLOSED_STR, NULL); b->lid_closed = true; @@ -201,7 +201,7 @@ static int button_dispatch(sd_event_source *s, int fd, uint32_t revents, void *u } else if (ev.code == SW_DOCK) { log_struct(LOG_INFO, LOG_MESSAGE("System docked."), - LOG_MESSAGE_ID(SD_MESSAGE_SYSTEM_DOCKED), + "MESSAGE_ID=" SD_MESSAGE_SYSTEM_DOCKED_STR, NULL); b->docked = true; @@ -212,7 +212,7 @@ static int button_dispatch(sd_event_source *s, int fd, uint32_t revents, void *u if (ev.code == SW_LID) { log_struct(LOG_INFO, LOG_MESSAGE("Lid opened."), - LOG_MESSAGE_ID(SD_MESSAGE_LID_OPENED), + "MESSAGE_ID=" SD_MESSAGE_LID_OPENED_STR, NULL); b->lid_closed = false; @@ -221,7 +221,7 @@ static int button_dispatch(sd_event_source *s, int fd, uint32_t revents, void *u } else if (ev.code == SW_DOCK) { log_struct(LOG_INFO, LOG_MESSAGE("System undocked."), - LOG_MESSAGE_ID(SD_MESSAGE_SYSTEM_UNDOCKED), + "MESSAGE_ID=" SD_MESSAGE_SYSTEM_UNDOCKED_STR, NULL); b->docked = false; diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c index 26b6c3a92..971b447a7 100644 --- a/src/login/logind-dbus.c +++ b/src/login/logind-dbus.c @@ -34,7 +34,7 @@ #include "escape.h" #include "fd-util.h" #include "fileio-label.h" -#include "formats-util.h" +#include "format-util.h" #include "fs-util.h" #include "logind.h" #include "mkdir.h" @@ -1301,8 +1301,7 @@ static int flush_devices(Manager *m) { } else { struct dirent *de; - while ((de = readdir(d))) { - + FOREACH_DIRENT_ALL(de, d, break) { if (!dirent_is_file(de)) continue; @@ -1447,7 +1446,7 @@ static int bus_manager_log_shutdown( p = strjoina(p, " (", m->wall_message, ")."); return log_struct(LOG_NOTICE, - LOG_MESSAGE_ID(SD_MESSAGE_SHUTDOWN), + "MESSAGE_ID=" SD_MESSAGE_SHUTDOWN_STR, p, q, NULL); @@ -2465,13 +2464,9 @@ static int method_set_wall_message( return 1; /* Will call us back */ #endif // 0 - if (isempty(wall_message)) - m->wall_message = mfree(m->wall_message); - else { - r = free_and_strdup(&m->wall_message, wall_message); - if (r < 0) - return log_oom(); - } + r = free_and_strdup(&m->wall_message, empty_to_null(wall_message)); + if (r < 0) + return log_oom(); m->enable_wall_messages = enable_wall_messages; diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c index 194ebc1b7..92fa3c9c6 100644 --- a/src/login/logind-seat.c +++ b/src/login/logind-seat.c @@ -27,7 +27,7 @@ #include "alloc-util.h" #include "fd-util.h" #include "fileio.h" -#include "formats-util.h" +#include "format-util.h" #include "logind-acl.h" #include "logind-seat.h" #include "mkdir.h" @@ -420,7 +420,7 @@ int seat_start(Seat *s) { return 0; log_struct(LOG_INFO, - LOG_MESSAGE_ID(SD_MESSAGE_SEAT_START), + "MESSAGE_ID=" SD_MESSAGE_SEAT_START_STR, "SEAT_ID=%s", s->id, LOG_MESSAGE("New seat %s.", s->id), NULL); @@ -450,7 +450,7 @@ int seat_stop(Seat *s, bool force) { if (s->started) log_struct(LOG_INFO, - LOG_MESSAGE_ID(SD_MESSAGE_SEAT_STOP), + "MESSAGE_ID=" SD_MESSAGE_SEAT_STOP_STR, "SEAT_ID=%s", s->id, LOG_MESSAGE("Removed seat %s.", s->id), NULL); @@ -547,8 +547,6 @@ int seat_attach_session(Seat *s, Session *session) { LIST_PREPEND(sessions_by_seat, s->sessions, session); seat_assign_position(s, session); - seat_send_changed(s, "Sessions", NULL); - /* On seats with VTs, the VT logic defines which session is active. On * seats without VTs, we automatically activate new sessions. */ if (!seat_has_vts(s)) diff --git a/src/login/logind-session.c b/src/login/logind-session.c index 9123d50fa..bd2aac99a 100644 --- a/src/login/logind-session.c +++ b/src/login/logind-session.c @@ -35,7 +35,7 @@ #include "escape.h" #include "fd-util.h" #include "fileio.h" -#include "formats-util.h" +#include "format-util.h" #include "io-util.h" #include "logind-session.h" #include "mkdir.h" @@ -592,7 +592,7 @@ int session_start(Session *s) { return r; log_struct(s->class == SESSION_BACKGROUND ? LOG_DEBUG : LOG_INFO, - LOG_MESSAGE_ID(SD_MESSAGE_SESSION_START), + "MESSAGE_ID=" SD_MESSAGE_SESSION_START_STR, "SESSION_ID=%s", s->id, "USER_ID=%s", s->user->name, "LEADER="PID_FMT, s->leader, @@ -617,12 +617,10 @@ int session_start(Session *s) { /* Send signals */ session_send_signal(s, true); - user_send_changed(s->user, "Sessions", "Display", NULL); + user_send_changed(s->user, "Display", NULL); if (s->seat) { if (s->seat->active == s) - seat_send_changed(s->seat, "Sessions", "ActiveSession", NULL); - else - seat_send_changed(s->seat, "Sessions", NULL); + seat_send_changed(s->seat, "ActiveSession", NULL); } return 0; @@ -720,7 +718,7 @@ int session_finalize(Session *s) { if (s->started) log_struct(s->class == SESSION_BACKGROUND ? LOG_DEBUG : LOG_INFO, - LOG_MESSAGE_ID(SD_MESSAGE_SESSION_STOP), + "MESSAGE_ID=" SD_MESSAGE_SESSION_STOP_STR, "SESSION_ID=%s", s->id, "USER_ID=%s", s->user->name, "LEADER="PID_FMT, s->leader, @@ -750,11 +748,10 @@ int session_finalize(Session *s) { seat_set_active(s->seat, NULL); seat_save(s->seat); - seat_send_changed(s->seat, "Sessions", NULL); } user_save(s->user); - user_send_changed(s->user, "Sessions", "Display", NULL); + user_send_changed(s->user, "Display", NULL); return 0; } diff --git a/src/sleep/sleep.c b/src/sleep/sleep.c index dc1d55d8b..22f4db613 100644 --- a/src/sleep/sleep.c +++ b/src/sleep/sleep.c @@ -112,7 +112,7 @@ static int execute(char **modes, char **states) { execute_directories(dirs, DEFAULT_TIMEOUT_USEC, arguments); log_struct(LOG_INFO, - LOG_MESSAGE_ID(SD_MESSAGE_SLEEP_START), + "MESSAGE_ID=" SD_MESSAGE_SLEEP_START_STR, LOG_MESSAGE("Suspending system..."), "SLEEP=%s", arg_verb, NULL); @@ -122,7 +122,7 @@ static int execute(char **modes, char **states) { return r; log_struct(LOG_INFO, - LOG_MESSAGE_ID(SD_MESSAGE_SLEEP_STOP), + "MESSAGE_ID=" SD_MESSAGE_SLEEP_STOP_STR, LOG_MESSAGE("System resumed."), "SLEEP=%s", arg_verb, NULL); -- 2.30.2