From: Mike Yuan Date: Thu, 13 Mar 2025 13:49:13 +0000 (+0100) Subject: fileio: modernize get_proc_field() X-Git-Tag: v258-rc1~704^2~8 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=15036f855534b24296ea76709030e257d1eb82a9;p=thirdparty%2Fsystemd.git fileio: modernize get_proc_field() - Drop effectively unused "terminator" param, imply whitespace - Make ret param optional - Return ENODATA if the requested key is not found, rather than ENOENT - Turn ENOENT -> ENOSYS if /proc/ is not mounted - Don't skip whitespaces before ':', nothing needs this handling anyways - Remove the special treatment for all "0"s. We don't actually use this for capabilities given pidref_get_capability() exists - Switch away from read_full_virtual_file() - files using "field" scheme under /proc/ seem all to be "seq_file"s (refer to da65941c3ee03495541c3bffbccc9012c8d9a5f8 for details on file types) --- diff --git a/src/basic/fileio.c b/src/basic/fileio.c index b3fdc05c127..c5eedc312b3 100644 --- a/src/basic/fileio.c +++ b/src/basic/fileio.c @@ -889,74 +889,46 @@ int script_get_shebang_interpreter(const char *path, char **ret) { return 0; } -/** - * Retrieve one field from a file like /proc/self/status. pattern - * should not include whitespace or the delimiter (':'). pattern matches only - * the beginning of a line. Whitespace before ':' is skipped. Whitespace and - * zeros after the ':' will be skipped. field must be freed afterwards. - * terminator specifies the terminating characters of the field value (not - * included in the value). - */ -int get_proc_field(const char *filename, const char *pattern, const char *terminator, char **field) { - _cleanup_free_ char *status = NULL; - char *t, *f; +int get_proc_field(const char *path, const char *key, char **ret) { + _cleanup_fclose_ FILE *f = NULL; int r; - assert(terminator); - assert(filename); - assert(pattern); - assert(field); + /* Retrieve one field from a file like /proc/self/status. "key" matches the beginning of the line + * and should not include whitespace or the delimiter (':'). + * Whitespaces after the ':' will be skipped. Only the first element is returned + * (i.e. for /proc/meminfo line "MemTotal: 1024 kB" -> return "1024"). */ + + assert(path); + assert(key); - r = read_full_virtual_file(filename, &status, NULL); + r = fopen_unlocked(path, "re", &f); + if (r == -ENOENT && proc_mounted() == 0) + return -ENOSYS; if (r < 0) return r; - t = status; - - do { - bool pattern_ok; - - do { - t = strstr(t, pattern); - if (!t) - return -ENOENT; - - /* Check that pattern occurs in beginning of line. */ - pattern_ok = (t == status || t[-1] == '\n'); - - t += strlen(pattern); - - } while (!pattern_ok); - - t += strspn(t, " \t"); - if (!*t) - return -ENOENT; - - } while (*t != ':'); - - t++; - - if (*t) { - t += strspn(t, " \t"); - - /* Also skip zeros, because when this is used for - * capabilities, we don't want the zeros. This way the - * same capability set always maps to the same string, - * irrespective of the total capability set size. For - * other numbers it shouldn't matter. */ - t += strspn(t, "0"); - /* Back off one char if there's nothing but whitespace - and zeros */ - if (!*t || isspace(*t)) - t--; + for (;;) { + _cleanup_free_ char *line = NULL; + + r = read_line(f, LONG_LINE_MAX, &line); + if (r < 0) + return r; + if (r == 0) + return -ENODATA; + + char *l = startswith(line, key); + if (l && *l == ':') { + if (ret) { + char *s = strdupcspn(skip_leading_chars(l + 1, " \t"), WHITESPACE); + if (!s) + return -ENOMEM; + + *ret = s; + } + + return 0; + } } - - f = strdupcspn(t, terminator); - if (!f) - return -ENOMEM; - - *field = f; - return 0; } DIR* xopendirat(int dir_fd, const char *name, int flags) { diff --git a/src/basic/fileio.h b/src/basic/fileio.h index 49da9a677c5..eff98802928 100644 --- a/src/basic/fileio.h +++ b/src/basic/fileio.h @@ -98,7 +98,7 @@ int verify_file_at(int dir_fd, const char *fn, const char *blob, bool accept_ext int script_get_shebang_interpreter(const char *path, char **ret); -int get_proc_field(const char *filename, const char *pattern, const char *terminator, char **field); +int get_proc_field(const char *path, const char *key, char **ret); DIR* xopendirat(int dir_fd, const char *name, int flags); diff --git a/src/basic/process-util.c b/src/basic/process-util.c index d3c43c3ba4e..1ee11d61239 100644 --- a/src/basic/process-util.c +++ b/src/basic/process-util.c @@ -870,7 +870,7 @@ int get_process_umask(pid_t pid, mode_t *ret) { p = procfs_file_alloca(pid, "status"); - r = get_proc_field(p, "Umask", WHITESPACE, &m); + r = get_proc_field(p, "Umask", &m); if (r == -ENOENT) return -ESRCH; if (r < 0) @@ -2079,9 +2079,9 @@ int get_process_threads(pid_t pid) { p = procfs_file_alloca(pid, "status"); - r = get_proc_field(p, "Threads", WHITESPACE, &t); + r = get_proc_field(p, "Threads", &t); if (r == -ENOENT) - return proc_mounted() == 0 ? -ENOSYS : -ESRCH; + return -ESRCH; if (r < 0) return r; diff --git a/src/basic/virt.c b/src/basic/virt.c index 575ac917b09..4c5c5b3b9aa 100644 --- a/src/basic/virt.c +++ b/src/basic/virt.c @@ -432,8 +432,8 @@ static Virtualization detect_vm_zvm(void) { _cleanup_free_ char *t = NULL; int r; - r = get_proc_field("/proc/sysinfo", "VM00 Control Program", WHITESPACE, &t); - if (r == -ENOENT) + r = get_proc_field("/proc/sysinfo", "VM00 Control Program", &t); + if (IN_SET(r, -ENOENT, -ENODATA)) return VIRTUALIZATION_NONE; if (r < 0) return r; @@ -655,7 +655,7 @@ Virtualization detect_container(void) { /* proot doesn't use PID namespacing, so we can just check if we have a matching tracer for this * invocation without worrying about it being elsewhere. */ - r = get_proc_field("/proc/self/status", "TracerPid", WHITESPACE, &p); + r = get_proc_field("/proc/self/status", "TracerPid", &p); if (r < 0) log_debug_errno(r, "Failed to read our own trace PID, ignoring: %m"); else if (!streq(p, "0")) { diff --git a/src/oom/oomd.c b/src/oom/oomd.c index f49accd7791..15cbb3a2c35 100644 --- a/src/oom/oomd.c +++ b/src/oom/oomd.c @@ -123,7 +123,7 @@ static int run(int argc, char *argv[]) { int fd = n == 1 ? SD_LISTEN_FDS_START : -EBADF; /* SwapTotal is always available in /proc/meminfo and defaults to 0, even on swap-disabled kernels. */ - r = get_proc_field("/proc/meminfo", "SwapTotal", WHITESPACE, &swap); + r = get_proc_field("/proc/meminfo", "SwapTotal", &swap); if (r < 0) return log_error_errno(r, "Failed to get SwapTotal from /proc/meminfo: %m"); diff --git a/src/shared/hibernate-util.c b/src/shared/hibernate-util.c index 2aca788fb8e..55ca4d5cf09 100644 --- a/src/shared/hibernate-util.c +++ b/src/shared/hibernate-util.c @@ -428,7 +428,7 @@ static int get_proc_meminfo_active(unsigned long long *ret) { assert(ret); - r = get_proc_field("/proc/meminfo", "Active(anon)", WHITESPACE, &active_str); + r = get_proc_field("/proc/meminfo", "Active(anon)", &active_str); if (r < 0) return log_debug_errno(r, "Failed to retrieve Active(anon) from /proc/meminfo: %m"); diff --git a/src/test/test-fileio.c b/src/test/test-fileio.c index 67943b65ba7..97eeb59afbb 100644 --- a/src/test/test-fileio.c +++ b/src/test/test-fileio.c @@ -333,19 +333,19 @@ TEST(script_get_shebang_interpreter) { } TEST(status_field) { - _cleanup_free_ char *p = NULL, *s = NULL, *z = NULL; + _cleanup_free_ char *p = NULL, *s = NULL; unsigned long long total = 0, buffers = 0; int r; - r = get_proc_field("/proc/meminfo", "MemTotal", WHITESPACE, &p); - if (r != -ENOENT) { + r = get_proc_field("/proc/meminfo", "MemTotal", &p); + if (!IN_SET(r, -ENOENT, -ENOSYS)) { assert_se(r == 0); puts(p); assert_se(safe_atollu(p, &total) == 0); } - r = get_proc_field("/proc/meminfo", "Buffers", WHITESPACE, &s); - if (r != -ENOENT) { + r = get_proc_field("/proc/meminfo", "Buffers", &s); + if (!IN_SET(r, -ENOENT, -ENOSYS)) { assert_se(r == 0); puts(s); assert_se(safe_atollu(s, &buffers) == 0); @@ -353,14 +353,6 @@ TEST(status_field) { if (p) assert_se(buffers < total); - - /* Seccomp should be a good test for field full of zeros. */ - r = get_proc_field("/proc/meminfo", "Seccomp", WHITESPACE, &z); - if (r != -ENOENT) { - assert_se(r == 0); - puts(z); - assert_se(safe_atollu(z, &buffers) == 0); - } } TEST(read_one_line_file) { diff --git a/src/test/test-seccomp.c b/src/test/test-seccomp.c index d006e8b7b08..39a89d1e8dd 100644 --- a/src/test/test-seccomp.c +++ b/src/test/test-seccomp.c @@ -382,7 +382,7 @@ TEST(protect_sysctl) { return; } - assert_se(get_proc_field("/proc/self/status", "Seccomp", WHITESPACE, &seccomp) == 0); + assert_se(get_proc_field("/proc/self/status", "Seccomp", &seccomp) == 0); if (!streq(seccomp, "0")) log_warning("Warning: seccomp filter detected, results may be unreliable for %s", __func__);