From 6bfd44ee04515ce0a506ab88b91ca332ca35dedd Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 11 Oct 2021 10:53:56 +0200 Subject: [PATCH 1/2] fileio: add read_virtual_file_at() flavour that takes dir_fd/path pair --- src/basic/fileio.c | 19 ++++++++++++++++--- src/basic/fileio.h | 5 ++++- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/basic/fileio.c b/src/basic/fileio.c index 0a483854f2a..09c72830c26 100644 --- a/src/basic/fileio.c +++ b/src/basic/fileio.c @@ -547,12 +547,25 @@ int read_virtual_file_fd(int fd, size_t max_size, char **ret_contents, size_t *r return !truncated; } -int read_virtual_file(const char *filename, size_t max_size, char **ret_contents, size_t *ret_size) { +int read_virtual_file_at( + int dir_fd, + const char *filename, + size_t max_size, + char **ret_contents, + size_t *ret_size) { + _cleanup_close_ int fd = -1; - assert(filename); + assert(dir_fd >= 0 || dir_fd == AT_FDCWD); - fd = open(filename, O_RDONLY | O_NOCTTY | O_CLOEXEC); + if (!filename) { + if (dir_fd == AT_FDCWD) + return -EBADF; + + return read_virtual_file_fd(dir_fd, max_size, ret_contents, ret_size); + } + + fd = openat(dir_fd, filename, O_RDONLY | O_NOCTTY | O_CLOEXEC); if (fd < 0) return -errno; diff --git a/src/basic/fileio.h b/src/basic/fileio.h index 899def946bd..cea3dd893d1 100644 --- a/src/basic/fileio.h +++ b/src/basic/fileio.h @@ -69,7 +69,10 @@ static inline int read_full_file(const char *filename, char **ret_contents, size } int read_virtual_file_fd(int fd, size_t max_size, char **ret_contents, size_t *ret_size); -int read_virtual_file(const char *filename, size_t max_size, char **ret_contents, size_t *ret_size); +int read_virtual_file_at(int dir_fd, const char *filename, size_t max_size, char **ret_contents, size_t *ret_size); +static inline int read_virtual_file(const char *filename, size_t max_size, char **ret_contents, size_t *ret_size) { + return read_virtual_file_at(AT_FDCWD, filename, max_size, ret_contents, ret_size); +} static inline int read_full_virtual_file(const char *filename, char **ret_contents, size_t *ret_size) { return read_virtual_file(filename, SIZE_MAX, ret_contents, ret_size); } From c19a51bec40ae5e5073464e72411e7920d05d683 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 11 Oct 2021 10:54:31 +0200 Subject: [PATCH 2/2] util: invert ac_power() source type check So far we assumed every power source was a battery except for the ones which definitely are not. I think this logic makes little sense, as "battery" is kinda the exceptional case here, not the other way round. Hence let's invert the type check, and denylist "Battery" devices rather than allowlist "Mains" devices. This should increase compatibility with alternative types of power sources, in particular USB ones. This takes into account that additional power types have been added since we wrote the original code, and in particular should cover the siutation discussed here OK: https://sources.debian.org/src/powermgmt-base/1.36/power_supply.txt/#L31 https://sources.debian.org/src/powermgmt-base/1.36/on_ac_power/#L25 Also, modernizes the code in various was ways. Inspired by and fixes: #20964 --- src/basic/util.c | 63 ++++++++++++++++++++++-------------------------- 1 file changed, 29 insertions(+), 34 deletions(-) diff --git a/src/basic/util.c b/src/basic/util.c index 955b18bd2aa..43e74b773fc 100644 --- a/src/basic/util.c +++ b/src/basic/util.c @@ -119,62 +119,57 @@ int on_ac_power(void) { bool found_offline = false, found_online = false; _cleanup_closedir_ DIR *d = NULL; struct dirent *de; + int r; d = opendir("/sys/class/power_supply"); if (!d) return errno == ENOENT ? true : -errno; FOREACH_DIRENT(de, d, return -errno) { - _cleanup_close_ int fd = -1, device = -1; - char contents[6]; - ssize_t n; + _cleanup_close_ int device_fd = -1; + _cleanup_free_ char *contents = NULL; + unsigned v; - device = openat(dirfd(d), de->d_name, O_DIRECTORY|O_RDONLY|O_CLOEXEC|O_NOCTTY); - if (device < 0) { + device_fd = openat(dirfd(d), de->d_name, O_DIRECTORY|O_RDONLY|O_CLOEXEC); + if (device_fd < 0) { if (IN_SET(errno, ENOENT, ENOTDIR)) continue; return -errno; } - fd = openat(device, "type", O_RDONLY|O_CLOEXEC|O_NOCTTY); - if (fd < 0) { - if (errno == ENOENT) - continue; + r = read_virtual_file_at(device_fd, "type", SIZE_MAX, &contents, NULL); + if (r == -ENOENT) + continue; + if (r < 0) + return r; - return -errno; - } + delete_trailing_chars(contents, NEWLINE); - n = read(fd, contents, sizeof(contents)); - if (n < 0) - return -errno; - - if (n != 6 || memcmp(contents, "Mains\n", 6)) + /* We assume every power source is AC, except for batteries. See + * https://github.com/torvalds/linux/blob/4eef766b7d4d88f0b984781bc1bcb574a6eafdc7/include/linux/power_supply.h#L176 + * for defined power source types. Also see: + * https://www.kernel.org/doc/Documentation/ABI/testing/sysfs-class-power */ + if (streq(contents, "Battery")) continue; - safe_close(fd); - fd = openat(device, "online", O_RDONLY|O_CLOEXEC|O_NOCTTY); - if (fd < 0) { - if (errno == ENOENT) - continue; + contents = mfree(contents); - return -errno; - } + r = read_virtual_file_at(device_fd, "online", SIZE_MAX, &contents, NULL); + if (r == -ENOENT) + continue; + if (r < 0) + return r; - n = read(fd, contents, sizeof(contents)); - if (n < 0) - return -errno; + delete_trailing_chars(contents, NEWLINE); - if (n != 2 || contents[1] != '\n') - return -EIO; - - if (contents[0] == '1') { + r = safe_atou(contents, &v); + if (r < 0) + return r; + if (v > 0) /* At least 1 and 2 are defined as different types of 'online' */ found_online = true; - break; - } else if (contents[0] == '0') - found_offline = true; else - return -EIO; + found_offline = true; } return found_online || !found_offline;