From 92663a5e5beb44d91a5cdae56d1bc7cfce01fe0a Mon Sep 17 00:00:00 2001 From: =?utf8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 10 May 2022 13:39:08 +0200 Subject: [PATCH] tree-wide: use LOG_MESSAGE() where possible Also break some long lines for more uniform formatting. No functional change. I went over all log_struct, log_struct_errno, log_unit_struct, log_unit_struct_errno calls, and they seem fine. --- src/basic/log.h | 3 ++- src/basic/user-util.c | 2 +- src/core/device.c | 3 ++- src/core/job.c | 14 +++++++------- src/core/service.c | 3 ++- src/login/logind-button.c | 12 ++++++++---- src/resolve/resolved-dns-server.c | 3 ++- src/resolve/resolved-dns-transaction.c | 3 ++- src/test/test-log.c | 15 ++++++++++----- src/timesync/timesyncd-manager.c | 3 ++- src/udev/udevadm-settle.c | 2 +- 11 files changed, 39 insertions(+), 24 deletions(-) diff --git a/src/basic/log.h b/src/basic/log.h index 0d927bfce97..6f0d838de37 100644 --- a/src/basic/log.h +++ b/src/basic/log.h @@ -298,7 +298,8 @@ int log_emergency_level(void); bool log_on_console(void) _pure_; -/* Helper to prepare various field for structured logging */ +/* Helper to wrap the main message in structured logging. The macro doesn't do much, + * except to provide visual grouping of the format string and its arguments. */ #define LOG_MESSAGE(fmt, ...) "MESSAGE=" fmt, ##__VA_ARGS__ void log_received_signal(int level, const struct signalfd_siginfo *si); diff --git a/src/basic/user-util.c b/src/basic/user-util.c index da3b922bc90..8b1283a9eca 100644 --- a/src/basic/user-util.c +++ b/src/basic/user-util.c @@ -794,7 +794,7 @@ bool valid_user_group_name(const char *u, ValidUserFlags flags) { /* Compare with strict result and warn if result doesn't match */ if (FLAGS_SET(flags, VALID_USER_WARN) && !valid_user_group_name(u, 0)) log_struct(LOG_NOTICE, - "MESSAGE=Accepting user/group name '%s', which does not match strict user/group name rules.", u, + LOG_MESSAGE("Accepting user/group name '%s', which does not match strict user/group name rules.", u), "USER_GROUP_NAME=%s", u, "MESSAGE_ID=" SD_MESSAGE_UNSAFE_USER_NAME_STR); diff --git a/src/core/device.c b/src/core/device.c index f65dbfcb1e7..feb6479361e 100644 --- a/src/core/device.c +++ b/src/core/device.c @@ -493,7 +493,8 @@ static int device_setup_unit(Manager *m, sd_device *dev, const char *path, bool LOG_WARNING, r, "MESSAGE_ID=" SD_MESSAGE_DEVICE_PATH_NOT_SUITABLE_STR, "DEVICE=%s", path, - LOG_MESSAGE("Failed to generate valid unit name from device path '%s', ignoring device: %m", path)); + LOG_MESSAGE("Failed to generate valid unit name from device path '%s', ignoring device: %m", + path)); u = manager_get_unit(m, e); if (u) { diff --git a/src/core/job.c b/src/core/job.c index 94ab3816264..36d6f0a4565 100644 --- a/src/core/job.c +++ b/src/core/job.c @@ -716,8 +716,8 @@ static void job_emit_done_message(Unit *u, uint32_t job_id, JobType t, JobResult log_unit_struct( u, job_done_messages[result].log_level, - "MESSAGE=%s was skipped because all trigger condition checks failed.", - ident, + LOG_MESSAGE("%s was skipped because all trigger condition checks failed.", + ident), "JOB_ID=%" PRIu32, job_id, "JOB_TYPE=%s", job_type_to_string(t), "JOB_RESULT=%s", job_result_to_string(result), @@ -727,11 +727,11 @@ static void job_emit_done_message(Unit *u, uint32_t job_id, JobType t, JobResult log_unit_struct( u, job_done_messages[result].log_level, - "MESSAGE=%s was skipped because of a failed condition check (%s=%s%s).", - ident, - condition_type_to_string(c->type), - c->negate ? "!" : "", - c->parameter, + LOG_MESSAGE("%s was skipped because of a failed condition check (%s=%s%s).", + ident, + condition_type_to_string(c->type), + c->negate ? "!" : "", + c->parameter), "JOB_ID=%" PRIu32, job_id, "JOB_TYPE=%s", job_type_to_string(t), "JOB_RESULT=%s", job_result_to_string(result), diff --git a/src/core/service.c b/src/core/service.c index 2d7a0868524..9f7af9dffb5 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -2365,7 +2365,8 @@ static void service_enter_restart(Service *s) { log_unit_struct(UNIT(s), LOG_INFO, "MESSAGE_ID=" SD_MESSAGE_UNIT_RESTART_SCHEDULED_STR, LOG_UNIT_INVOCATION_ID(UNIT(s)), - LOG_UNIT_MESSAGE(UNIT(s), "Scheduled restart job, restart counter is at %u.", s->n_restarts), + LOG_UNIT_MESSAGE(UNIT(s), + "Scheduled restart job, restart counter is at %u.", s->n_restarts), "N_RESTARTS=%u", s->n_restarts); /* Notify clients about changed restart counter */ diff --git a/src/login/logind-button.c b/src/login/logind-button.c index a2b43d36843..368a08a0952 100644 --- a/src/login/logind-button.c +++ b/src/login/logind-button.c @@ -226,7 +226,8 @@ static int button_dispatch(sd_event_source *s, int fd, uint32_t revents, void *u log_debug("Power key pressed. Further action depends on the key press duration."); start_long_press(b->manager, &b->manager->power_key_long_press_event_source, long_press_of_power_key_handler); } else { - log_struct(LOG_INFO, LOG_MESSAGE("Power key pressed short."), + log_struct(LOG_INFO, + LOG_MESSAGE("Power key pressed short."), "MESSAGE_ID=" SD_MESSAGE_POWER_KEY_STR); manager_handle_action(b->manager, INHIBIT_HANDLE_POWER_KEY, b->manager->handle_power_key, b->manager->power_key_ignore_inhibited, true); } @@ -242,7 +243,8 @@ static int button_dispatch(sd_event_source *s, int fd, uint32_t revents, void *u log_debug("Reboot key pressed. Further action depends on the key press duration."); start_long_press(b->manager, &b->manager->reboot_key_long_press_event_source, long_press_of_reboot_key_handler); } else { - log_struct(LOG_INFO, LOG_MESSAGE("Reboot key pressed short."), + log_struct(LOG_INFO, + LOG_MESSAGE("Reboot key pressed short."), "MESSAGE_ID=" SD_MESSAGE_REBOOT_KEY_STR); manager_handle_action(b->manager, INHIBIT_HANDLE_REBOOT_KEY, b->manager->handle_reboot_key, b->manager->reboot_key_ignore_inhibited, true); } @@ -259,7 +261,8 @@ static int button_dispatch(sd_event_source *s, int fd, uint32_t revents, void *u log_debug("Suspend key pressed. Further action depends on the key press duration."); start_long_press(b->manager, &b->manager->suspend_key_long_press_event_source, long_press_of_suspend_key_handler); } else { - log_struct(LOG_INFO, LOG_MESSAGE("Suspend key pressed short."), + log_struct(LOG_INFO, + LOG_MESSAGE("Suspend key pressed short."), "MESSAGE_ID=" SD_MESSAGE_SUSPEND_KEY_STR); manager_handle_action(b->manager, INHIBIT_HANDLE_SUSPEND_KEY, b->manager->handle_suspend_key, b->manager->suspend_key_ignore_inhibited, true); } @@ -270,7 +273,8 @@ static int button_dispatch(sd_event_source *s, int fd, uint32_t revents, void *u log_debug("Hibernate key pressed. Further action depends on the key press duration."); start_long_press(b->manager, &b->manager->hibernate_key_long_press_event_source, long_press_of_hibernate_key_handler); } else { - log_struct(LOG_INFO, LOG_MESSAGE("Hibernate key pressed short."), + log_struct(LOG_INFO, + LOG_MESSAGE("Hibernate key pressed short."), "MESSAGE_ID=" SD_MESSAGE_HIBERNATE_KEY_STR); manager_handle_action(b->manager, INHIBIT_HANDLE_HIBERNATE_KEY, b->manager->handle_hibernate_key, b->manager->hibernate_key_ignore_inhibited, true); } diff --git a/src/resolve/resolved-dns-server.c b/src/resolve/resolved-dns-server.c index 2ef2568dac1..9b74a8d6d86 100644 --- a/src/resolve/resolved-dns-server.c +++ b/src/resolve/resolved-dns-server.c @@ -726,7 +726,8 @@ void dns_server_warn_downgrade(DnsServer *server) { log_struct(LOG_NOTICE, "MESSAGE_ID=" SD_MESSAGE_DNSSEC_DOWNGRADE_STR, - LOG_MESSAGE("Server %s does not support DNSSEC, downgrading to non-DNSSEC mode.", strna(dns_server_string_full(server))), + LOG_MESSAGE("Server %s does not support DNSSEC, downgrading to non-DNSSEC mode.", + strna(dns_server_string_full(server))), "DNS_SERVER=%s", strna(dns_server_string_full(server)), "DNS_SERVER_FEATURE_LEVEL=%s", dns_server_feature_level_to_string(server->possible_feature_level)); diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index cec3cea7c58..cd9cdeb7832 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -394,7 +394,8 @@ void dns_transaction_complete(DnsTransaction *t, DnsTransactionState state) { log_struct(LOG_NOTICE, "MESSAGE_ID=" SD_MESSAGE_DNSSEC_FAILURE_STR, - LOG_MESSAGE("DNSSEC validation failed for question %s: %s", key_str, dnssec_result_to_string(t->answer_dnssec_result)), + LOG_MESSAGE("DNSSEC validation failed for question %s: %s", + key_str, dnssec_result_to_string(t->answer_dnssec_result)), "DNS_TRANSACTION=%" PRIu16, t->id, "DNS_QUESTION=%s", key_str, "DNSSEC_RESULT=%s", dnssec_result_to_string(t->answer_dnssec_result), diff --git a/src/test/test-log.c b/src/test/test-log.c index cf438e6869f..ae3d073d827 100644 --- a/src/test/test-log.c +++ b/src/test/test-log.c @@ -31,16 +31,21 @@ static void test_log_struct(void) { "MESSAGE=Waldo PID="PID_FMT" (no errno)", getpid_cached(), "SERVICE=piepapo"); - log_struct_errno(LOG_INFO, EILSEQ, - "MESSAGE=Waldo PID="PID_FMT": %m (normal)", getpid_cached(), + /* The same as above, just using LOG_MESSAGE(), which is generally recommended */ + log_struct(LOG_INFO, + LOG_MESSAGE("Waldo PID="PID_FMT" (no errno)", getpid_cached()), "SERVICE=piepapo"); + log_struct_errno(LOG_INFO, EILSEQ, + LOG_MESSAGE("Waldo PID="PID_FMT": %m (normal)", getpid_cached()), + "SERVICE=piepapo"); + log_struct_errno(LOG_INFO, SYNTHETIC_ERRNO(EILSEQ), - "MESSAGE=Waldo PID="PID_FMT": %m (synthetic)", getpid_cached(), - "SERVICE=piepapo"); + LOG_MESSAGE("Waldo PID="PID_FMT": %m (synthetic)", getpid_cached()), + "SERVICE=piepapo"); log_struct(LOG_INFO, - "MESSAGE=Foobar PID="PID_FMT, getpid_cached(), + LOG_MESSAGE("Foobar PID="PID_FMT, getpid_cached()), "FORMAT_STR_TEST=1=%i A=%c 2=%hi 3=%li 4=%lli 1=%p foo=%s 2.5=%g 3.5=%g 4.5=%Lg", (int) 1, 'A', (short) 2, (long int) 3, (long long int) 4, (void*) 1, "foo", (float) 2.5f, (double) 3.5, (long double) 4.5, "SUFFIX=GOT IT"); diff --git a/src/timesync/timesyncd-manager.c b/src/timesync/timesyncd-manager.c index 78b9cb99321..9325523838b 100644 --- a/src/timesync/timesyncd-manager.c +++ b/src/timesync/timesyncd-manager.c @@ -620,7 +620,8 @@ static int manager_receive_response(sd_event_source *source, int fd, uint32_t re m->synchronized = true; log_struct(LOG_INFO, - LOG_MESSAGE("Initial clock synchronization to %s.", FORMAT_TIMESTAMP_STYLE(dts.realtime, TIMESTAMP_US)), + LOG_MESSAGE("Initial clock synchronization to %s.", + FORMAT_TIMESTAMP_STYLE(dts.realtime, TIMESTAMP_US)), "MESSAGE_ID=" SD_MESSAGE_TIME_SYNC_STR, "MONOTONIC_USEC=" USEC_FMT, dts.monotonic, "REALTIME_USEC=" USEC_FMT, dts.realtime, diff --git a/src/udev/udevadm-settle.c b/src/udev/udevadm-settle.c index 6da9439bd28..df9fefc8d00 100644 --- a/src/udev/udevadm-settle.c +++ b/src/udev/udevadm-settle.c @@ -150,7 +150,7 @@ static int emit_deprecation_warning(void) { return -ENOMEM; log_struct(LOG_NOTICE, - "MESSAGE=systemd-udev-settle.service is deprecated. Please fix %s not to pull it in.", t, + LOG_MESSAGE("systemd-udev-settle.service is deprecated. Please fix %s not to pull it in.", t), "OFFENDING_UNITS=%s", t, "MESSAGE_ID=" SD_MESSAGE_SYSTEMD_UDEV_SETTLE_DEPRECATED_STR); } -- 2.39.2