From bb56c27fc81da2777cd7064a0b88ca011eced509 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 18 Oct 2024 19:41:25 +0200 Subject: [PATCH 1/7] sysv-generator: break long message into lines The journal handles multi-line messages nicely, and they are easier to read. Drop the recycling symbol, there is no circular process here, we go from a to b and never back to a again. --- src/sysv-generator/sysv-generator.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/sysv-generator/sysv-generator.c b/src/sysv-generator/sysv-generator.c index f3b447095a..fde8c057e3 100644 --- a/src/sysv-generator/sysv-generator.c +++ b/src/sysv-generator/sysv-generator.c @@ -763,12 +763,13 @@ static int enumerate_sysv(const LookupPaths *lp, Hashmap *all_services) { return log_oom(); log_struct(LOG_WARNING, - LOG_MESSAGE("SysV service '%s' lacks a native systemd unit file. " - "%s Automatically generating a unit file for compatibility. Please update package to include a native systemd unit file, in order to make it safe, robust and future-proof. " + LOG_MESSAGE("SysV service '%s' lacks a native systemd unit file, " + "automatically generating a unit file for compatibility.\n" + "Please update package to include a native systemd unit file.\n" "%s This compatibility logic is deprecated, expect removal soon. %s", fpath, - special_glyph(SPECIAL_GLYPH_RECYCLING), - special_glyph(SPECIAL_GLYPH_WARNING_SIGN), special_glyph(SPECIAL_GLYPH_WARNING_SIGN)), + special_glyph(SPECIAL_GLYPH_WARNING_SIGN), + special_glyph(SPECIAL_GLYPH_WARNING_SIGN)), "MESSAGE_ID=" SD_MESSAGE_SYSV_GENERATOR_DEPRECATED_STR, "SYSVSCRIPT=%s", fpath, "UNIT=%s", name); From f0764b98e5c136cb948a8034949064f610acca24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 28 Oct 2024 13:15:32 +0100 Subject: [PATCH 2/7] qrcode-util: add debug message to show why a qrcode wasn't printed --- src/shared/qrcode-util.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/shared/qrcode-util.c b/src/shared/qrcode-util.c index 36cd5dcd7c..56eace395e 100644 --- a/src/shared/qrcode-util.c +++ b/src/shared/qrcode-util.c @@ -179,8 +179,10 @@ int print_qrcode_full(FILE *out, const char *header, const char *string, unsigne /* If this is not a UTF-8 system or ANSI colors aren't supported/disabled don't print any QR * codes */ - if (!is_locale_utf8() || !colors_enabled()) - return -EOPNOTSUPP; + if (!is_locale_utf8()) + return log_debug_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), "Not an UTF-8 system, cannot print qrcode"); + if (!colors_enabled()) + return log_debug_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), "Colors are disabled, cannot print qrcode"); r = dlopen_qrencode(); if (r < 0) @@ -188,21 +190,21 @@ int print_qrcode_full(FILE *out, const char *header, const char *string, unsigne qr = sym_QRcode_encodeString(string, 0, QR_ECLEVEL_L, QR_MODE_8, 1); if (!qr) - return -ENOMEM; + return log_oom_debug(); if (row != UINT_MAX && column != UINT_MAX) { - int fd; unsigned qr_code_width, qr_code_height; - fd = fileno(out); + + int fd = fileno(out); if (fd < 0) return log_debug_errno(errno, "Failed to get file descriptor from the file stream: %m"); - qr_code_width = qr_code_height = qr->width + 8; + qr_code_width = qr_code_height = qr->width + 8; if (column + qr_code_width > tty_width) column = tty_width - qr_code_width; /* Terminal characters are twice as high as they are wide so it's qr_code_height / 2, - * our QR code prints an extra new line, so we have -1 as well */ + * our QR code prints an extra new line, so we have -1 as well */ if (row + qr_code_height > tty_height) row = tty_height - (qr_code_height / 2 ) - 1; @@ -218,7 +220,6 @@ int print_qrcode_full(FILE *out, const char *header, const char *string, unsigne fprintf(out, "\n%s:\n\n", header); write_qrcode(out, qr, row, column); - fputc('\n', out); sym_QRcode_free(qr); From 5a64c86936477ecea5cc1fb8dbc79faf522cf370 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 28 Oct 2024 13:38:58 +0100 Subject: [PATCH 3/7] bsod: do not check for color support When invoked on a running system, bsod would not print the qrcode. The check for "color support" on stdout is pointless, since we're not printing to stdout but to a terminal fd that is opened separately. --- src/journal/bsod.c | 4 +++- src/shared/qrcode-util.c | 13 +++++++++++-- src/shared/qrcode-util.h | 33 ++++++++++++++++++++++++--------- 3 files changed, 38 insertions(+), 12 deletions(-) diff --git a/src/journal/bsod.c b/src/journal/bsod.c index cdf9e4874c..c76b04718e 100644 --- a/src/journal/bsod.c +++ b/src/journal/bsod.c @@ -212,7 +212,9 @@ static int display_emergency_message_fullscreen(const char *message) { goto cleanup; } - r = print_qrcode_full(stream, "Scan the QR code", message, qr_code_start_row, qr_code_start_column, w.ws_col, w.ws_row); + r = print_qrcode_full(stream, "Scan the QR code", + message, qr_code_start_row, qr_code_start_column, w.ws_col, w.ws_row, + /* check_tty= */ false); if (r < 0) log_warning_errno(r, "QR code could not be printed, ignoring: %m"); diff --git a/src/shared/qrcode-util.c b/src/shared/qrcode-util.c index 56eace395e..afb2875aa0 100644 --- a/src/shared/qrcode-util.c +++ b/src/shared/qrcode-util.c @@ -173,7 +173,16 @@ static void write_qrcode(FILE *output, QRcode *qr, unsigned int row, unsigned in fflush(output); } -int print_qrcode_full(FILE *out, const char *header, const char *string, unsigned row, unsigned column, unsigned tty_width, unsigned tty_height) { +int print_qrcode_full( + FILE *out, + const char *header, + const char *string, + unsigned row, + unsigned column, + unsigned tty_width, + unsigned tty_height, + bool check_tty) { + QRcode* qr; int r; @@ -181,7 +190,7 @@ int print_qrcode_full(FILE *out, const char *header, const char *string, unsigne * codes */ if (!is_locale_utf8()) return log_debug_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), "Not an UTF-8 system, cannot print qrcode"); - if (!colors_enabled()) + if (check_tty && !colors_enabled()) return log_debug_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), "Colors are disabled, cannot print qrcode"); r = dlopen_qrencode(); diff --git a/src/shared/qrcode-util.h b/src/shared/qrcode-util.h index ee58294436..89a15bb3f5 100644 --- a/src/shared/qrcode-util.h +++ b/src/shared/qrcode-util.h @@ -1,6 +1,7 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ - #pragma once + +#include #include #include #include @@ -8,15 +9,29 @@ #if HAVE_QRENCODE int dlopen_qrencode(void); -int print_qrcode_full(FILE *out, const char *header, const char *string, unsigned row, unsigned column, unsigned tty_width, unsigned tty_height); -static inline int print_qrcode(FILE *out, const char *header, const char *string) { - return print_qrcode_full(out, header, string, UINT_MAX, UINT_MAX, UINT_MAX, UINT_MAX); -} +int print_qrcode_full( + FILE *out, + const char *header, + const char *string, + unsigned row, + unsigned column, + unsigned tty_width, + unsigned tty_height, + bool check_tty); #else -static inline int print_qrcode_full(FILE *out, const char *header, const char *string, unsigned row, unsigned column, unsigned tty_width, unsigned tty_height) { - return -EOPNOTSUPP; -} -static inline int print_qrcode(FILE *out, const char *header, const char *string) { +static inline int print_qrcode_full( + FILE *out, + const char *header, + const char *string, + unsigned row, + unsigned column, + unsigned tty_width, + unsigned tty_height, + bool check_tty) { return -EOPNOTSUPP; } #endif + +static inline int print_qrcode(FILE *out, const char *header, const char *string) { + return print_qrcode_full(out, header, string, UINT_MAX, UINT_MAX, UINT_MAX, UINT_MAX, true); +} From b137b2979868e2de5fb5c26e90bacee33597b8e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 28 Oct 2024 13:39:36 +0100 Subject: [PATCH 4/7] test-terminal-util: print value of colors_enabled() This makes it easier to diagnose why colors are disabled. --- src/test/test-terminal-util.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/test/test-terminal-util.c b/src/test/test-terminal-util.c index ad800a4111..9586e4e245 100644 --- a/src/test/test-terminal-util.c +++ b/src/test/test-terminal-util.c @@ -25,6 +25,10 @@ "in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat " \ "non proident, sunt in culpa qui officia deserunt mollit anim id est laborum." +TEST(colors_enabled) { + log_info("colors_enabled: %s", yes_no(colors_enabled())); +} + TEST(default_term_for_tty) { puts(default_term_for_tty("/dev/tty23")); puts(default_term_for_tty("/dev/ttyS23")); From 439306da8b3962f683f5359c461d1669c070f377 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 28 Oct 2024 13:45:40 +0100 Subject: [PATCH 5/7] qrcode-util: avoid memleak in error path --- src/shared/qrcode-util.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/shared/qrcode-util.c b/src/shared/qrcode-util.c index afb2875aa0..da88d80d03 100644 --- a/src/shared/qrcode-util.c +++ b/src/shared/qrcode-util.c @@ -173,6 +173,8 @@ static void write_qrcode(FILE *output, QRcode *qr, unsigned int row, unsigned in fflush(output); } +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(QRcode*, sym_QRcode_free, NULL); + int print_qrcode_full( FILE *out, const char *header, @@ -183,7 +185,6 @@ int print_qrcode_full( unsigned tty_height, bool check_tty) { - QRcode* qr; int r; /* If this is not a UTF-8 system or ANSI colors aren't supported/disabled don't print any QR @@ -197,7 +198,8 @@ int print_qrcode_full( if (r < 0) return r; - qr = sym_QRcode_encodeString(string, 0, QR_ECLEVEL_L, QR_MODE_8, 1); + _cleanup_(sym_QRcode_freep) QRcode *qr = + sym_QRcode_encodeString(string, 0, QR_ECLEVEL_L, QR_MODE_8, 1); if (!qr) return log_oom_debug(); @@ -231,7 +233,6 @@ int print_qrcode_full( write_qrcode(out, qr, row, column); fputc('\n', out); - sym_QRcode_free(qr); return 0; } #endif From abf1cae0a75ca07f09afbb4eaa9f11fc429b1d02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 28 Oct 2024 13:51:25 +0100 Subject: [PATCH 6/7] bsod: make message for qrcode more useful People know what a qrcode is. We don't need to tell them to scan it. Instead, we should say what the code contains. While at it, rename "stream" to "f" in line with the usual style. --- src/journal/bsod.c | 8 ++++---- test/units/TEST-04-JOURNAL.bsod.sh | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/journal/bsod.c b/src/journal/bsod.c index c76b04718e..2f06808cd4 100644 --- a/src/journal/bsod.c +++ b/src/journal/bsod.c @@ -143,7 +143,7 @@ static int display_emergency_message_fullscreen(const char *message) { unsigned qr_code_start_row = 1, qr_code_start_column = 1; char ttybuf[STRLEN("/dev/tty") + DECIMAL_STR_MAX(int) + 1]; _cleanup_close_ int fd = -EBADF; - _cleanup_fclose_ FILE *stream = NULL; + _cleanup_fclose_ FILE *f = NULL; char read_character_buffer = '\0'; struct winsize w = { .ws_col = 80, @@ -206,13 +206,13 @@ static int display_emergency_message_fullscreen(const char *message) { goto cleanup; } - r = fdopen_independent(fd, "r+", &stream); + r = fdopen_independent(fd, "r+", &f); if (r < 0) { r = log_error_errno(errno, "Failed to open output file: %m"); goto cleanup; } - r = print_qrcode_full(stream, "Scan the QR code", + r = print_qrcode_full(f, "Scan the error message", message, qr_code_start_row, qr_code_start_column, w.ws_col, w.ws_row, /* check_tty= */ false); if (r < 0) @@ -228,7 +228,7 @@ static int display_emergency_message_fullscreen(const char *message) { goto cleanup; } - r = read_one_char(stream, &read_character_buffer, USEC_INFINITY, NULL); + r = read_one_char(f, &read_character_buffer, USEC_INFINITY, NULL); if (r < 0 && r != -EINTR) log_error_errno(r, "Failed to read character: %m"); diff --git a/test/units/TEST-04-JOURNAL.bsod.sh b/test/units/TEST-04-JOURNAL.bsod.sh index 802d474f16..d4a4ff26da 100755 --- a/test/units/TEST-04-JOURNAL.bsod.sh +++ b/test/units/TEST-04-JOURNAL.bsod.sh @@ -89,7 +89,7 @@ journalctl --sync SYSTEMD_COLORS=256 /usr/lib/systemd/systemd-bsod & PID=$! vcs_dump_and_check "Root emergency message" -grep -aq "Scan the QR code" /tmp/console.dump +grep -aq "Scan the error message" /tmp/console.dump # TODO: check if systemd-bsod exits on a key press (didn't figure this one out yet) kill $PID timeout 10 bash -c "while kill -0 $PID; do sleep .5; done" From 10faa40ba781cf499258a3b37de02dd643822dc6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 28 Oct 2024 13:59:05 +0100 Subject: [PATCH 7/7] cryptenroll,homectl,journalctl: adjust messages before qrcodes Users will generally know what a qrcode is, so let's not treat them as dumb and explain that it can be scanned. OTOH, we should say what the qrcode contains and it is useful to give a hint why the users would want to scan it. Reword messages accordingly. (Also, don't say "to your phone", when somebody might be using a stolen phone, or something else then a phone.) --- src/cryptenroll/cryptenroll-recovery.c | 2 +- src/home/homectl-recovery-key.c | 2 +- src/journal/journalctl-authenticate.c | 4 +--- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/cryptenroll/cryptenroll-recovery.c b/src/cryptenroll/cryptenroll-recovery.c index 0a5e9cdcbe..33b58eeb37 100644 --- a/src/cryptenroll/cryptenroll-recovery.c +++ b/src/cryptenroll/cryptenroll-recovery.c @@ -67,7 +67,7 @@ int enroll_recovery( "whenever authentication is requested.\n", stderr); fflush(stderr); - (void) print_qrcode(stderr, "You may optionally scan the recovery key off screen", password); + (void) print_qrcode(stderr, "Optionally scan the recovery key for safekeeping", password); if (asprintf(&keyslot_as_string, "%i", keyslot) < 0) { r = log_oom(); diff --git a/src/home/homectl-recovery-key.c b/src/home/homectl-recovery-key.c index b5c57e14a8..2b76303edd 100644 --- a/src/home/homectl-recovery-key.c +++ b/src/home/homectl-recovery-key.c @@ -160,7 +160,7 @@ int identity_add_recovery_key(sd_json_variant **v) { "whenever authentication is requested.\n", stderr); fflush(stderr); - (void) print_qrcode(stderr, "You may optionally scan the recovery key off screen", password); + (void) print_qrcode(stderr, "Optionally scan the recovery key for safekeeping", password); return 0; } diff --git a/src/journal/journalctl-authenticate.c b/src/journal/journalctl-authenticate.c index 8167aef7f5..865814cd03 100644 --- a/src/journal/journalctl-authenticate.c +++ b/src/journal/journalctl-authenticate.c @@ -199,9 +199,7 @@ int action_setup_keys(void) { if (!url) return log_oom(); - (void) print_qrcode(stderr, - "To transfer the verification key to your phone scan the QR code below", - url); + (void) print_qrcode(stderr, "Scan the verification key to transfer it to another device", url); #endif return 0;