From fd6ec01d283dc64d836386d002121ec26c66c701 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 20 Jun 2023 12:22:28 +0200 Subject: [PATCH 1/9] sleep-config: rename can_sleep_state() parameter Just some trivial renaming, to indicate that these are not the supported but the requested sleep states. --- src/shared/sleep-config.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/shared/sleep-config.c b/src/shared/sleep-config.c index dcc233ca6f..cba5993afe 100644 --- a/src/shared/sleep-config.c +++ b/src/shared/sleep-config.c @@ -510,11 +510,11 @@ int check_wakeup_type(void) { return false; } -int can_sleep_state(char **types) { +int can_sleep_state(char **requested_types) { _cleanup_free_ char *text = NULL; int r; - if (strv_isempty(types)) + if (strv_isempty(requested_types)) return true; /* If /sys is read-only we cannot sleep */ @@ -530,13 +530,13 @@ int can_sleep_state(char **types) { } const char *found; - r = string_contains_word_strv(text, NULL, types, &found); + r = string_contains_word_strv(text, NULL, requested_types, &found); if (r < 0) return log_debug_errno(r, "Failed to parse /sys/power/state: %m"); if (r > 0) log_debug("Sleep mode \"%s\" is supported by the kernel.", found); else if (DEBUG_LOGGING) { - _cleanup_free_ char *t = strv_join(types, "/"); + _cleanup_free_ char *t = strv_join(requested_types, "/"); log_debug("Sleep mode %s not supported by the kernel, sorry.", strnull(t)); } return r; From 1cf78c8f785a492e592479fae2824fe99ece9b40 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 20 Jun 2023 12:23:58 +0200 Subject: [PATCH 2/9] sleep-config: replace useless fstat() by useful fd_verify_regular() For some reason there was an fstat() call here whose results was entirely ignored. Let's remove it. Let's add a call to fd_verify_regular() instead, because this is a code path for swap files, hence let's make sure we actually operate on a file, and nothing else. --- src/shared/sleep-config.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/shared/sleep-config.c b/src/shared/sleep-config.c index cba5993afe..cee0a69cb0 100644 --- a/src/shared/sleep-config.c +++ b/src/shared/sleep-config.c @@ -641,7 +641,6 @@ static int swap_device_to_device_id(const SwapEntry *swap, dev_t *ret_dev) { static int calculate_swap_file_offset(const SwapEntry *swap, uint64_t *ret_offset) { _cleanup_close_ int fd = -EBADF; _cleanup_free_ struct fiemap *fiemap = NULL; - struct stat sb; int r; assert(swap); @@ -652,8 +651,9 @@ static int calculate_swap_file_offset(const SwapEntry *swap, uint64_t *ret_offse if (fd < 0) return log_debug_errno(errno, "Failed to open swap file %s to determine on-disk offset: %m", swap->device); - if (fstat(fd, &sb) < 0) - return log_debug_errno(errno, "Failed to stat %s: %m", swap->device); + r = fd_verify_regular(fd); + if (r < 0) + return log_debug_errno(r, "Selected swap file is not a regular file."); r = fd_is_fs_type(fd, BTRFS_SUPER_MAGIC); if (r < 0) From 08e0ed300ad369e0389dfd9335ef6d91b4f8501a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 20 Jun 2023 12:26:46 +0200 Subject: [PATCH 3/9] sleep-config: check if we operate on regular file in swap_device_to_device_id() in swap file code path Similar to the previous commit: before we continue doing swap file operations let's ensure this actually is a swap file. --- src/shared/sleep-config.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/shared/sleep-config.c b/src/shared/sleep-config.c index cee0a69cb0..0db02fae87 100644 --- a/src/shared/sleep-config.c +++ b/src/shared/sleep-config.c @@ -631,6 +631,10 @@ static int swap_device_to_device_id(const SwapEntry *swap, dev_t *ret_dev) { return 0; } + r = stat_verify_regular(&sb); + if (r < 0) + return r; + return get_block_device(swap->device, ret_dev); } From 1296b427a7516f56752163c7f52b76a36408e36d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 20 Jun 2023 12:31:10 +0200 Subject: [PATCH 4/9] sleep-config: replace SwapEntry's .type field with a proper enum Following our usual rule: let's parse this early into internal representation, and stick to that. don't pass unparsed strings around needlessly. --- src/shared/sleep-config.c | 37 +++++++++++++++++++++++++------------ src/shared/sleep-config.h | 9 ++++++++- src/sleep/sleep.c | 8 +++----- 3 files changed, 36 insertions(+), 18 deletions(-) diff --git a/src/shared/sleep-config.c b/src/shared/sleep-config.c index 0db02fae87..88e907422b 100644 --- a/src/shared/sleep-config.c +++ b/src/shared/sleep-config.c @@ -597,7 +597,6 @@ SwapEntry* swap_entry_free(SwapEntry *se) { return NULL; free(se->device); - free(se->type); return mfree(se); } @@ -623,7 +622,7 @@ static int swap_device_to_device_id(const SwapEntry *swap, dev_t *ret_dev) { if (r < 0) return -errno; - if (streq(swap->type, "partition")) { + if (swap->type == SWAP_BLOCK) { if (!S_ISBLK(sb.st_mode)) return -ENOTBLK; @@ -649,7 +648,8 @@ static int calculate_swap_file_offset(const SwapEntry *swap, uint64_t *ret_offse assert(swap); assert(swap->device); - assert(streq(swap->type, "file")); + assert(swap->type == SWAP_FILE); + assert(ret_offset); fd = open(swap->device, O_RDONLY|O_CLOEXEC|O_NOCTTY); if (fd < 0) @@ -678,10 +678,13 @@ static int calculate_swap_file_offset(const SwapEntry *swap, uint64_t *ret_offse static int read_resume_files(dev_t *ret_resume, uint64_t *ret_resume_offset) { _cleanup_free_ char *resume_str = NULL, *resume_offset_str = NULL; - uint64_t resume_offset = 0; + uint64_t resume_offset; dev_t resume; int r; + assert(ret_resume); + assert(ret_resume_offset); + r = read_one_line_file("/sys/power/resume", &resume_str); if (r < 0) return log_debug_errno(r, "Error reading /sys/power/resume: %m"); @@ -691,9 +694,10 @@ static int read_resume_files(dev_t *ret_resume, uint64_t *ret_resume_offset) { return log_debug_errno(r, "Error parsing /sys/power/resume device: %s: %m", resume_str); r = read_one_line_file("/sys/power/resume_offset", &resume_offset_str); - if (r == -ENOENT) + if (r == -ENOENT) { log_debug_errno(r, "Kernel does not support resume_offset; swap file offset detection will be skipped."); - else if (r < 0) + resume_offset = 0; + } else if (r < 0) return log_debug_errno(r, "Error reading /sys/power/resume_offset: %m"); else { r = safe_atou64(resume_offset_str, &resume_offset); @@ -707,7 +711,6 @@ static int read_resume_files(dev_t *ret_resume, uint64_t *ret_resume_offset) { *ret_resume = resume; *ret_resume_offset = resume_offset; - return 0; } @@ -766,20 +769,25 @@ int find_hibernate_location(HibernateLocation **ret_hibernate_location) { (void) fscanf(f, "%*s %*s %*s %*s %*s\n"); for (unsigned i = 1;; i++) { _cleanup_(swap_entry_freep) SwapEntry *swap = NULL; + _cleanup_free_ char *type = NULL; uint64_t swap_offset = 0; int k; - swap = new0(SwapEntry, 1); + swap = new(SwapEntry, 1); if (!swap) return -ENOMEM; + *swap = (SwapEntry) { + .type = _SWAP_TYPE_INVALID, + }; + k = fscanf(f, "%ms " /* device/file */ "%ms " /* type of swap */ "%" PRIu64 /* swap size */ "%" PRIu64 /* used */ "%i\n", /* priority */ - &swap->device, &swap->type, &swap->size, &swap->used, &swap->priority); + &swap->device, &type, &swap->size, &swap->used, &swap->priority); if (k == EOF) break; if (k != 5) { @@ -787,17 +795,20 @@ int find_hibernate_location(HibernateLocation **ret_hibernate_location) { continue; } - if (streq(swap->type, "file")) { + if (streq(type, "file")) { + if (endswith(swap->device, "\\040(deleted)")) { log_debug("Ignoring deleted swap file '%s'.", swap->device); continue; } + swap->type = SWAP_FILE; + r = calculate_swap_file_offset(swap, &swap_offset); if (r < 0) return r; - } else if (streq(swap->type, "partition")) { + } else if (streq(type, "partition")) { const char *fn; fn = path_startswith(swap->device, "/dev/"); @@ -806,8 +817,10 @@ int find_hibernate_location(HibernateLocation **ret_hibernate_location) { continue; } + swap->type = SWAP_BLOCK; + } else { - log_debug("%s: swap type %s is unsupported for hibernation, ignoring", swap->device, swap->type); + log_debug("%s: swap type %s is unsupported for hibernation, ignoring", swap->device, type); continue; } diff --git a/src/shared/sleep-config.h b/src/shared/sleep-config.h index b8245b129f..126377bcfd 100644 --- a/src/shared/sleep-config.h +++ b/src/shared/sleep-config.h @@ -28,10 +28,17 @@ typedef struct SleepConfig { SleepConfig* free_sleep_config(SleepConfig *sc); DEFINE_TRIVIAL_CLEANUP_FUNC(SleepConfig*, free_sleep_config); +typedef enum SwapType { + SWAP_BLOCK, + SWAP_FILE, + _SWAP_TYPE_MAX, + _SWAP_TYPE_INVALID = -EINVAL, +} SwapType; + /* entry in /proc/swaps */ typedef struct SwapEntry { char *device; - char *type; + SwapType type; uint64_t size; uint64_t used; int priority; diff --git a/src/sleep/sleep.c b/src/sleep/sleep.c index 0dc3ad3558..685339e942 100644 --- a/src/sleep/sleep.c +++ b/src/sleep/sleep.c @@ -63,12 +63,10 @@ static int write_hibernate_location_info(const HibernateLocation *hibernate_loca log_debug("Wrote resume= value for %s to /sys/power/resume: %s", hibernate_location->swap->device, resume_str); /* if it's a swap partition, we're done */ - if (streq(hibernate_location->swap->type, "partition")) - return r; + if (hibernate_location->swap->type == SWAP_BLOCK) + return 0; - if (!streq(hibernate_location->swap->type, "file")) - return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), - "Invalid hibernate type: %s", hibernate_location->swap->type); + assert(hibernate_location->swap->type == SWAP_FILE); /* Only available in 4.17+ */ if (hibernate_location->offset > 0 && access("/sys/power/resume_offset", W_OK) < 0) { From 98034eb0ea2f38ee6688273f5f1e33a31a3b514b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 20 Jun 2023 12:32:21 +0200 Subject: [PATCH 5/9] sleep-config: pin swap device while operating via O_PATH --- src/shared/sleep-config.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/shared/sleep-config.c b/src/shared/sleep-config.c index 88e907422b..342eb28388 100644 --- a/src/shared/sleep-config.c +++ b/src/shared/sleep-config.c @@ -611,15 +611,18 @@ HibernateLocation* hibernate_location_free(HibernateLocation *hl) { } static int swap_device_to_device_id(const SwapEntry *swap, dev_t *ret_dev) { + _cleanup_close_ int fd = -EBADF; struct stat sb; int r; assert(swap); assert(swap->device); - assert(swap->type); - r = stat(swap->device, &sb); - if (r < 0) + fd = open(swap->device, O_CLOEXEC|O_PATH); + if (fd < 0) + return -errno; + + if (fstat(fd, &sb) < 0) return -errno; if (swap->type == SWAP_BLOCK) { @@ -634,7 +637,7 @@ static int swap_device_to_device_id(const SwapEntry *swap, dev_t *ret_dev) { if (r < 0) return r; - return get_block_device(swap->device, ret_dev); + return get_block_device_fd(fd, ret_dev); } /* From 3595a9b7b8b77a912320548f1d87369d109ce774 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 20 Jun 2023 12:34:09 +0200 Subject: [PATCH 6/9] sleep-config: don't use 'device_id' moniker for a dev_t entity We usually call dev_t entities "devnum" or "devno". That's redundant enough, let's not call this "device_id". In particular as that's something else (in udev context). --- src/shared/sleep-config.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/shared/sleep-config.c b/src/shared/sleep-config.c index 342eb28388..cf8bf3df67 100644 --- a/src/shared/sleep-config.c +++ b/src/shared/sleep-config.c @@ -610,7 +610,7 @@ HibernateLocation* hibernate_location_free(HibernateLocation *hl) { return mfree(hl); } -static int swap_device_to_device_id(const SwapEntry *swap, dev_t *ret_dev) { +static int swap_device_to_devnum(const SwapEntry *swap, dev_t *ret_dev) { _cleanup_close_ int fd = -EBADF; struct stat sb; int r; @@ -841,11 +841,11 @@ int find_hibernate_location(HibernateLocation **ret_hibernate_location) { } } - dev_t swap_device; - r = swap_device_to_device_id(swap, &swap_device); + dev_t swap_devno; + r = swap_device_to_devnum(swap, &swap_devno); if (r < 0) return log_debug_errno(r, "%s: failed to query device number: %m", swap->device); - if (swap_device == 0) + if (swap_devno == 0) return log_debug_errno(SYNTHETIC_ERRNO(ENODEV), "%s: not backed by block device.", swap->device); hibernate_location = hibernate_location_free(hibernate_location); @@ -854,7 +854,7 @@ int find_hibernate_location(HibernateLocation **ret_hibernate_location) { return -ENOMEM; *hibernate_location = (HibernateLocation) { - .devno = swap_device, + .devno = swap_devno, .offset = swap_offset, .swap = TAKE_PTR(swap), }; From 8601ecbc7cdcdcc734d33d867c714b75497f819a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 20 Jun 2023 13:21:28 +0200 Subject: [PATCH 7/9] sleep-config: rename .device field to .path This has been badly named given the path doesn't refer to a device quite likely, but to a path to a regular file. Hence let's be more precise with naming. (.device kinda suggests this was an sd_device object of sorts, but it really isn't.) --- src/shared/sleep-config.c | 48 +++++++++++++++++++-------------------- src/shared/sleep-config.h | 2 +- src/sleep/sleep.c | 10 ++++---- 3 files changed, 30 insertions(+), 30 deletions(-) diff --git a/src/shared/sleep-config.c b/src/shared/sleep-config.c index cf8bf3df67..6c7ce4b66a 100644 --- a/src/shared/sleep-config.c +++ b/src/shared/sleep-config.c @@ -596,7 +596,7 @@ SwapEntry* swap_entry_free(SwapEntry *se) { if (!se) return NULL; - free(se->device); + free(se->path); return mfree(se); } @@ -616,9 +616,9 @@ static int swap_device_to_devnum(const SwapEntry *swap, dev_t *ret_dev) { int r; assert(swap); - assert(swap->device); + assert(swap->path); - fd = open(swap->device, O_CLOEXEC|O_PATH); + fd = open(swap->path, O_CLOEXEC|O_PATH); if (fd < 0) return -errno; @@ -650,13 +650,13 @@ static int calculate_swap_file_offset(const SwapEntry *swap, uint64_t *ret_offse int r; assert(swap); - assert(swap->device); + assert(swap->path); assert(swap->type == SWAP_FILE); assert(ret_offset); - fd = open(swap->device, O_RDONLY|O_CLOEXEC|O_NOCTTY); + fd = open(swap->path, O_RDONLY|O_CLOEXEC|O_NOCTTY); if (fd < 0) - return log_debug_errno(errno, "Failed to open swap file %s to determine on-disk offset: %m", swap->device); + return log_debug_errno(errno, "Failed to open swap file %s to determine on-disk offset: %m", swap->path); r = fd_verify_regular(fd); if (r < 0) @@ -664,16 +664,16 @@ static int calculate_swap_file_offset(const SwapEntry *swap, uint64_t *ret_offse r = fd_is_fs_type(fd, BTRFS_SUPER_MAGIC); if (r < 0) - return log_debug_errno(r, "Error checking %s for Btrfs filesystem: %m", swap->device); + return log_debug_errno(r, "Error checking %s for Btrfs filesystem: %m", swap->path); if (r > 0) { - log_debug("%s: detection of swap file offset on Btrfs is not supported", swap->device); + log_debug("%s: detection of swap file offset on Btrfs is not supported", swap->path); *ret_offset = UINT64_MAX; return 0; } r = read_fiemap(fd, &fiemap); if (r < 0) - return log_debug_errno(r, "Unable to read extent map for '%s': %m", swap->device); + return log_debug_errno(r, "Unable to read extent map for '%s': %m", swap->path); *ret_offset = fiemap->fm_extents[0].fe_physical / page_size(); return 0; @@ -785,12 +785,12 @@ int find_hibernate_location(HibernateLocation **ret_hibernate_location) { }; k = fscanf(f, - "%ms " /* device/file */ + "%ms " /* device/file path */ "%ms " /* type of swap */ "%" PRIu64 /* swap size */ "%" PRIu64 /* used */ "%i\n", /* priority */ - &swap->device, &type, &swap->size, &swap->used, &swap->priority); + &swap->path, &type, &swap->size, &swap->used, &swap->priority); if (k == EOF) break; if (k != 5) { @@ -800,8 +800,8 @@ int find_hibernate_location(HibernateLocation **ret_hibernate_location) { if (streq(type, "file")) { - if (endswith(swap->device, "\\040(deleted)")) { - log_debug("Ignoring deleted swap file '%s'.", swap->device); + if (endswith(swap->path, "\\040(deleted)")) { + log_debug("Ignoring deleted swap file '%s'.", swap->path); continue; } @@ -814,29 +814,29 @@ int find_hibernate_location(HibernateLocation **ret_hibernate_location) { } else if (streq(type, "partition")) { const char *fn; - fn = path_startswith(swap->device, "/dev/"); + fn = path_startswith(swap->path, "/dev/"); if (fn && startswith(fn, "zram")) { - log_debug("%s: ignoring zram swap", swap->device); + log_debug("%s: ignoring zram swap", swap->path); continue; } swap->type = SWAP_BLOCK; } else { - log_debug("%s: swap type %s is unsupported for hibernation, ignoring", swap->device, type); + log_debug("%s: swap type %s is unsupported for hibernation, ignoring", swap->path, type); continue; } /* prefer resume device or highest priority swap with most remaining space */ if (sys_resume == 0) { if (hibernate_location && swap->priority < hibernate_location->swap->priority) { - log_debug("%s: ignoring device with lower priority", swap->device); + log_debug("%s: ignoring device with lower priority", swap->path); continue; } if (hibernate_location && (swap->priority == hibernate_location->swap->priority && swap->size - swap->used < hibernate_location->swap->size - hibernate_location->swap->used)) { - log_debug("%s: ignoring device with lower usable space", swap->device); + log_debug("%s: ignoring device with lower usable space", swap->path); continue; } } @@ -844,9 +844,9 @@ int find_hibernate_location(HibernateLocation **ret_hibernate_location) { dev_t swap_devno; r = swap_device_to_devnum(swap, &swap_devno); if (r < 0) - return log_debug_errno(r, "%s: failed to query device number: %m", swap->device); + return log_debug_errno(r, "%s: failed to query device number: %m", swap->path); if (swap_devno == 0) - return log_debug_errno(SYNTHETIC_ERRNO(ENODEV), "%s: not backed by block device.", swap->device); + return log_debug_errno(SYNTHETIC_ERRNO(ENODEV), "%s: not backed by block device.", swap->path); hibernate_location = hibernate_location_free(hibernate_location); hibernate_location = new(HibernateLocation, 1); @@ -861,12 +861,12 @@ int find_hibernate_location(HibernateLocation **ret_hibernate_location) { /* if the swap is the resume device, stop the loop */ if (location_is_resume_device(hibernate_location, sys_resume, sys_offset)) { - log_debug("%s: device matches configured resume settings.", hibernate_location->swap->device); + log_debug("%s: device matches configured resume settings.", hibernate_location->swap->path); resume_match = true; break; } - log_debug("%s: is a candidate device.", hibernate_location->swap->device); + log_debug("%s: is a candidate device.", hibernate_location->swap->path); } /* We found nothing at all */ @@ -888,11 +888,11 @@ int find_hibernate_location(HibernateLocation **ret_hibernate_location) { if (resume_match) log_debug("Hibernation will attempt to use swap entry with path: %s, device: %u:%u, offset: %" PRIu64 ", priority: %i", - hibernate_location->swap->device, major(hibernate_location->devno), minor(hibernate_location->devno), + hibernate_location->swap->path, major(hibernate_location->devno), minor(hibernate_location->devno), hibernate_location->offset, hibernate_location->swap->priority); else log_debug("/sys/power/resume is not configured; attempting to hibernate with path: %s, device: %u:%u, offset: %" PRIu64 ", priority: %i", - hibernate_location->swap->device, major(hibernate_location->devno), minor(hibernate_location->devno), + hibernate_location->swap->path, major(hibernate_location->devno), minor(hibernate_location->devno), hibernate_location->offset, hibernate_location->swap->priority); *ret_hibernate_location = TAKE_PTR(hibernate_location); diff --git a/src/shared/sleep-config.h b/src/shared/sleep-config.h index 126377bcfd..0f32e7f8ae 100644 --- a/src/shared/sleep-config.h +++ b/src/shared/sleep-config.h @@ -37,7 +37,7 @@ typedef enum SwapType { /* entry in /proc/swaps */ typedef struct SwapEntry { - char *device; + char *path; SwapType type; uint64_t size; uint64_t used; diff --git a/src/sleep/sleep.c b/src/sleep/sleep.c index 685339e942..4a6a777ed6 100644 --- a/src/sleep/sleep.c +++ b/src/sleep/sleep.c @@ -58,9 +58,9 @@ static int write_hibernate_location_info(const HibernateLocation *hibernate_loca r = write_string_file("/sys/power/resume", resume_str, WRITE_STRING_FILE_DISABLE_BUFFER); if (r < 0) return log_debug_errno(r, "Failed to write partition device to /sys/power/resume for '%s': '%s': %m", - hibernate_location->swap->device, resume_str); + hibernate_location->swap->path, resume_str); - log_debug("Wrote resume= value for %s to /sys/power/resume: %s", hibernate_location->swap->device, resume_str); + log_debug("Wrote resume= value for %s to /sys/power/resume: %s", hibernate_location->swap->path, resume_str); /* if it's a swap partition, we're done */ if (hibernate_location->swap->type == SWAP_BLOCK) @@ -72,7 +72,7 @@ static int write_hibernate_location_info(const HibernateLocation *hibernate_loca if (hibernate_location->offset > 0 && access("/sys/power/resume_offset", W_OK) < 0) { if (errno == ENOENT) { log_debug("Kernel too old, can't configure resume_offset for %s, ignoring: %" PRIu64, - hibernate_location->swap->device, hibernate_location->offset); + hibernate_location->swap->path, hibernate_location->offset); return 0; } @@ -83,9 +83,9 @@ static int write_hibernate_location_info(const HibernateLocation *hibernate_loca r = write_string_file("/sys/power/resume_offset", offset_str, WRITE_STRING_FILE_DISABLE_BUFFER); if (r < 0) return log_debug_errno(r, "Failed to write swap file offset to /sys/power/resume_offset for '%s': '%s': %m", - hibernate_location->swap->device, offset_str); + hibernate_location->swap->path, offset_str); - log_debug("Wrote resume_offset= value for %s to /sys/power/resume_offset: %s", hibernate_location->swap->device, offset_str); + log_debug("Wrote resume_offset= value for %s to /sys/power/resume_offset: %s", hibernate_location->swap->path, offset_str); return 0; } From c1583ca16a8dc65c9252a26f58ea3c7fc8e23c95 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 20 Jun 2023 14:15:42 +0200 Subject: [PATCH 8/9] sleep-config: reduce scope of DMI object path a bit We need this in a single function only, hence move it there, and make it a static field so that it has local scope. While we are at it, rename s/readsize to buf/bufsize, to make relationship clear. In particular as the data read is actually binary and "s" hence a misnomer, since it suggests it was a string. --- src/shared/sleep-config.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/shared/sleep-config.c b/src/shared/sleep-config.c index 6c7ce4b66a..04e76ad317 100644 --- a/src/shared/sleep-config.c +++ b/src/shared/sleep-config.c @@ -46,7 +46,6 @@ #define DISCHARGE_RATE_FILEPATH "/var/lib/systemd/sleep/battery_discharge_percentage_rate_per_hour" #define BATTERY_DISCHARGE_RATE_HASH_KEY SD_ID128_MAKE(5f,9a,20,18,38,76,46,07,8d,36,58,0b,bb,c4,e0,63) -#define SYS_ENTRY_RAW_FILE_TYPE1 "/sys/firmware/dmi/entries/1-0/raw" static void *CAPACITY_TO_PTR(int capacity) { assert(capacity >= 0); @@ -478,25 +477,25 @@ int battery_trip_point_alarm_exists(void) { /* Return true if wakeup type is APM timer */ int check_wakeup_type(void) { - _cleanup_free_ char *s = NULL; + static const char dmi_object_path[] = "/sys/firmware/dmi/entries/1-0/raw"; uint8_t wakeup_type_byte, tablesize; - size_t readsize; + _cleanup_free_ char *buf = NULL; + size_t bufsize; int r; /* implementation via dmi/entries */ - r = read_full_virtual_file(SYS_ENTRY_RAW_FILE_TYPE1, &s, &readsize); + r = read_full_virtual_file(dmi_object_path, &buf, &bufsize); if (r < 0) - return log_debug_errno(r, "Unable to read %s: %m", SYS_ENTRY_RAW_FILE_TYPE1); - - if (readsize < 25) - return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "Only read %zu bytes from %s (expected 25)", readsize, SYS_ENTRY_RAW_FILE_TYPE1); + return log_debug_errno(r, "Unable to read %s: %m", dmi_object_path); + if (bufsize < 25) + return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "Only read %zu bytes from %s (expected 25)", bufsize, dmi_object_path); /* index 1 stores the size of table */ - tablesize = (uint8_t) s[1]; + tablesize = (uint8_t) buf[1]; if (tablesize < 25) return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "Table size lesser than the index[0x18] where waketype byte is available."); - wakeup_type_byte = (uint8_t) s[24]; + wakeup_type_byte = (uint8_t) buf[24]; /* 0 is Reserved and 8 is AC Power Restored. As per table 12 in * https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_3.4.0.pdf */ if (wakeup_type_byte >= 128) From 4014172a8bc0838e483dbf62faf49e0862cadfeb Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 20 Jun 2023 14:18:52 +0200 Subject: [PATCH 9/9] =?UTF-8?q?sleep:=20rename=20sleep-config.[ch]=20?= =?UTF-8?q?=E2=86=92=20sleep-util.[ch]?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The file long ceased to be exclusively about configuration of the sleep operation. It contains many many calls for other purposes, hence give it a more generic name. --- src/login/logind-action.h | 2 +- src/login/logind-dbus.c | 2 +- src/shared/meson.build | 2 +- src/shared/{sleep-config.c => sleep-util.c} | 2 +- src/shared/{sleep-config.h => sleep-util.h} | 0 src/sleep/sleep.c | 2 +- src/test/test-sleep.c | 2 +- 7 files changed, 6 insertions(+), 6 deletions(-) rename src/shared/{sleep-config.c => sleep-util.c} (99%) rename src/shared/{sleep-config.h => sleep-util.h} (100%) diff --git a/src/login/logind-action.h b/src/login/logind-action.h index dfef6400e9..7cb94d9bea 100644 --- a/src/login/logind-action.h +++ b/src/login/logind-action.h @@ -24,7 +24,7 @@ typedef struct HandleActionData HandleActionData; #include "logind-inhibit.h" #include "logind.h" -#include "sleep-config.h" +#include "sleep-util.h" static inline bool handle_action_valid(HandleAction a) { return a >= 0 && a < _HANDLE_ACTION_MAX; diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c index 79531d2cc4..0d4ab28b4d 100644 --- a/src/login/logind-dbus.c +++ b/src/login/logind-dbus.c @@ -46,7 +46,7 @@ #include "process-util.h" #include "reboot-util.h" #include "selinux-util.h" -#include "sleep-config.h" +#include "sleep-util.h" #include "special.h" #include "serialize.h" #include "stdio-util.h" diff --git a/src/shared/meson.build b/src/shared/meson.build index d34e2c1dd9..5a982f3184 100644 --- a/src/shared/meson.build +++ b/src/shared/meson.build @@ -142,7 +142,7 @@ shared_sources = files( 'selinux-util.c', 'serialize.c', 'service-util.c', - 'sleep-config.c', + 'sleep-util.c', 'smack-util.c', 'socket-label.c', 'socket-netlink.c', diff --git a/src/shared/sleep-config.c b/src/shared/sleep-util.c similarity index 99% rename from src/shared/sleep-config.c rename to src/shared/sleep-util.c index 04e76ad317..eb9a43f507 100644 --- a/src/shared/sleep-config.c +++ b/src/shared/sleep-util.c @@ -36,7 +36,7 @@ #include "macro.h" #include "path-util.h" #include "siphash24.h" -#include "sleep-config.h" +#include "sleep-util.h" #include "stat-util.h" #include "stdio-util.h" #include "string-table.h" diff --git a/src/shared/sleep-config.h b/src/shared/sleep-util.h similarity index 100% rename from src/shared/sleep-config.h rename to src/shared/sleep-util.h diff --git a/src/sleep/sleep.c b/src/sleep/sleep.c index 4a6a777ed6..46f46914b5 100644 --- a/src/sleep/sleep.c +++ b/src/sleep/sleep.c @@ -34,7 +34,7 @@ #include "main-func.h" #include "parse-util.h" #include "pretty-print.h" -#include "sleep-config.h" +#include "sleep-util.h" #include "special.h" #include "stdio-util.h" #include "string-util.h" diff --git a/src/test/test-sleep.c b/src/test/test-sleep.c index a873e6eb1b..6bb8ca0874 100644 --- a/src/test/test-sleep.c +++ b/src/test/test-sleep.c @@ -12,7 +12,7 @@ #include "fd-util.h" #include "log.h" #include "memory-util.h" -#include "sleep-config.h" +#include "sleep-util.h" #include "strv.h" #include "tests.h"