From 092c05717cd59b569eddd07950e57a08a0f0c8b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 19 Apr 2021 08:37:39 +0200 Subject: [PATCH 1/3] udev/cdrom_id: use random_u64_range() and tweak log messages https://github.com/systemd/systemd/pull/19317#pullrequestreview-637765222 --- src/udev/cdrom_id/cdrom_id.c | 47 ++++++++++++++++++------------------ 1 file changed, 23 insertions(+), 24 deletions(-) diff --git a/src/udev/cdrom_id/cdrom_id.c b/src/udev/cdrom_id/cdrom_id.c index 2523752043..6ed5c4c64a 100644 --- a/src/udev/cdrom_id/cdrom_id.c +++ b/src/udev/cdrom_id/cdrom_id.c @@ -308,17 +308,16 @@ static int cd_media_compat(Context *c) { r = ioctl(c->fd, CDROM_DRIVE_STATUS, CDSL_CURRENT); if (r < 0) - return log_debug_errno(errno, "ioctl(CDROM_DRIVE_STATUS) failed: m"); - if (r != CDS_DISC_OK) { - log_debug("ioctl(CDROM_DRIVE_STATUS) → %d (%s), ignoring", - r, - r == CDS_NO_INFO ? "no info" : - r == CDS_NO_DISC ? "no disc" : - r == CDS_TRAY_OPEN ? "tray open" : - r == CDS_DRIVE_NOT_READY ? "drive not ready" : - "unkown status"); - return -ENOMEDIUM; - } + return log_debug_errno(errno, "ioctl(CDROM_DRIVE_STATUS) failed: %m"); + if (r != CDS_DISC_OK) + log_debug_errno(SYNTHETIC_ERRNO(ENOMEDIUM), + "ioctl(CDROM_DRIVE_STATUS) → %d (%s), ignoring.", + r, + r == CDS_NO_INFO ? "no info" : + r == CDS_NO_DISC ? "no disc" : + r == CDS_TRAY_OPEN ? "tray open" : + r == CDS_DRIVE_NOT_READY ? "drive not ready" : + "unkown status"); c->has_media = true; return 0; @@ -340,7 +339,7 @@ static int cd_inquiry(Context *c) { return r; if ((inq[0] & 0x1F) != 5) - return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "Not an MMC unit"); + return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "Not an MMC unit."); log_debug("INQUIRY: [%.8s][%.16s][%.4s]", inq + 8, inq + 16, inq + 32); return 0; @@ -450,7 +449,7 @@ static int cd_profiles(Context *c) { log_debug("GET CONFIGURATION: size of features buffer %zu", len); if (len > sizeof(features)) { - log_debug("cannot get features in a single query, truncating"); + log_debug("Cannot get features in a single query, truncating."); len = sizeof(features); } else if (len <= 8) len = sizeof(features); @@ -470,7 +469,7 @@ static int cd_profiles(Context *c) { log_debug("GET CONFIGURATION: size of features buffer %zu", len); if (len > sizeof(features)) { - log_debug("cannot get features in a single query, truncating"); + log_debug("Cannot get features in a single query, truncating."); len = sizeof(features); } @@ -529,7 +528,7 @@ static int dvd_ram_media_update_state(Context *c) { if (dvdstruct[4] & 0x02) { c->media_state = MEDIA_STATE_COMPLETE; - log_debug("write-protected DVD-RAM media inserted"); + log_debug("Write-protected DVD-RAM media inserted"); return 1; } @@ -545,24 +544,24 @@ static int dvd_ram_media_update_state(Context *c) { len = format[3]; if (len & 7 || len < 16) return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), - "invalid format capacities length"); + "Invalid format capacities length."); switch(format[8] & 3) { case 1: /* This means that last format was interrupted or failed, blank dvd-ram discs are * factory formatted. Take no action here as it takes quite a while to reformat a * dvd-ram and it's not automatically started. */ - log_debug("unformatted DVD-RAM media inserted"); + log_debug("Unformatted DVD-RAM media inserted."); return 1; case 2: - log_debug("formatted DVD-RAM media inserted"); + log_debug("Formatted DVD-RAM media inserted."); return 0; case 3: c->has_media = false; return log_debug_errno(SYNTHETIC_ERRNO(ENOMEDIUM), - "format capacities returned no media"); + "Format capacities returned no media."); } return 0; @@ -599,17 +598,17 @@ static int dvd_media_update_state(Context *c) { for (size_t offset = 32768; offset < 32768 + 2048; offset++) if (buffer[offset] != 0) { - log_debug("data in block 16, assuming complete"); + log_debug("Data in block 16, assuming complete."); return 0; } for (size_t offset = 0; offset < 2048; offset++) if (buffer[offset] != 0) { - log_debug("data in block 0, assuming complete"); + log_debug("Data in block 0, assuming complete."); return 0; } - log_debug("no data in blocks 0 or 16, assuming blank"); + log_debug("No data in blocks 0 or 16, assuming blank."); c->media_state = MEDIA_STATE_BLANK; return 0; } @@ -755,7 +754,7 @@ static int open_drive(Context *c) { if (++cnt >= 20 || errno != EBUSY) return log_debug_errno(errno, "Unable to open '%s': %m", arg_node); - (void) usleep(100 * USEC_PER_MSEC + random_u64() % (100 * USEC_PER_MSEC)); + (void) usleep(100 * USEC_PER_MSEC + random_u64_range(100 * USEC_PER_MSEC)); } log_debug("probing: '%s'", arg_node); @@ -831,7 +830,7 @@ static void print_feature(Feature feature, const char *prefix) { found = typesafe_bsearch(&in, feature_to_string, ELEMENTSOF(feature_to_string), feature_to_string_compare_func); if (!found) - return (void) log_debug("Unknown feature 0x%02x, ignoring", (unsigned) feature); + return (void) log_debug("Unknown feature 0x%02x, ignoring.", (unsigned) feature); printf("%s_%s=1\n", prefix, found->str); } From 19cdcd5dca233aa85051cacad14af127545541ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 19 Apr 2021 08:37:57 +0200 Subject: [PATCH 2/3] logind: tweak log message https://github.com/systemd/systemd/pull/19317#pullrequestreview-637765222 --- src/login/logind.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/login/logind.c b/src/login/logind.c index f461f03ec4..b782938a5c 100644 --- a/src/login/logind.c +++ b/src/login/logind.c @@ -268,7 +268,7 @@ static int manager_enumerate_seats(Manager *m) { s = hashmap_get(m->seats, de->d_name); if (!s) { if (unlinkat(dirfd(d), de->d_name, 0) < 0) - log_warning_errno(errno, "Failed to remove /run/systemd/seats/%s: %m", + log_warning_errno(errno, "Failed to remove /run/systemd/seats/%s, ignoring: %m", de->d_name); continue; } From 59e8042ecd893fdfef553a66dbb820395a504c2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 19 Apr 2021 08:43:36 +0200 Subject: [PATCH 3/3] basic/log: change error for unset errno to ESTRPIPE EIO is used in other contexts, so change ot something otherwise unused. ENOTRECOVERABLE was also suggested, but it's used in various other places in the code. https://github.com/systemd/systemd/pull/19317#discussion_r614887661 --- src/basic/log.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/basic/log.h b/src/basic/log.h index 81330166a4..51ba3d8fde 100644 --- a/src/basic/log.h +++ b/src/basic/log.h @@ -195,7 +195,7 @@ void log_assert_failed_return( _e = (log_get_max_level() >= LOG_PRI(_level)) \ ? log_internal(_level, _e, PROJECT_FILE, __LINE__, __func__, __VA_ARGS__) \ : -ERRNO_VALUE(_e); \ - _e < 0 ? _e : -EIO; \ + _e < 0 ? _e : -ESTRPIPE; \ }) #if BUILD_MODE_DEVELOPER && !defined(TEST_CODE)