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 b3fdc05c127c45df93c99c619cae377192206eed..c5eedc312b384a2948cdbb0b532743bd716ec5de 100644 (file)
--- 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 49da9a677c594a2272e283d56ab145b6aa4a2032..eff98802928df85e9e373bf9441c52cae779d951 100644 (file)
--- 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 d3c43c3ba4e784157f6002d6643c8911d58c56ac..1ee11d612393ab2d2989dd19e017a24e6e795502 100644 (file)
--- 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 575ac917b09e5ff6425cb847f38bcfc65f1b2aec..4c5c5b3b9aa5e625184ebb6361b2aaebbd3b864f 100644 (file)
--- 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 f49accd779174f7c411907dd9cef2da070263524..15cbb3a2c35b75655373ff8d62e32d5f5424d15d 100644 (file)
--- 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 2aca788fb8ee191fff1233196d6e94abb4fd1754..55ca4d5cf09f0bfea4d230eae56d8a375ac76855 100644 (file)
--- 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 67943b65ba77923c5b57f8eff57f63caa050670f..97eeb59afbb642f58a7362aefd19f2bbc631a69b 100644 (file)
--- 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 d006e8b7b0870e8c59bbb8a5b1c945117ad2cd29..39a89d1e8dd92084491aad4b4bdec58eb226b1de 100644 (file)
--- 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__);