shared: rework fd_lookup_path() helper

Reduce the stack and heap memory usage of the helper by using a caller
provided buffer. This allows us to avoid copying the path, yet again,
within the caller.

Aside: readlink(3) returns -1 on all errors, it won't return the
prospective size if the output buffer is too small.

v2:
 - swap fd_lookup_path() path/pathlen argument order

Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
Link: https://github.com/kmod-project/kmod/pull/345
Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>

diff --git a/shared/tmpfile-util.c b/shared/tmpfile-util.c
index 1548d2d0853c0ba822e0cf707428c2e37caf9b98..709e4840d07a8a80145920aa4f65a0668af5a272 100644 (file)
--- a/shared/tmpfile-util.c
+++ b/shared/tmpfile-util.c
@@ -20,21 +20,21 @@
 
 FILE *tmpfile_openat(int dirfd, mode_t mode, struct tmpfile *file)
 {
-       const char *tmpname_tmpl = "tmpfileXXXXXX";
+       const char *tmpname_tmpl = "/tmpfileXXXXXX";
        const char *tmpname;
        char tmpfile_path[PATH_MAX];
        int fd, n;
-       _cleanup_free_ char *targetdir;
        FILE *fp;
 
-       targetdir = fd_lookup_path(dirfd);
-       if (targetdir == NULL)
+       n = fd_lookup_path(dirfd, tmpfile_path, sizeof(tmpfile_path));
+       if (n < 0)
                goto create_fail;
 
-       n = snprintf(tmpfile_path, sizeof(tmpfile_path), "%s/%s", targetdir, tmpname_tmpl);
-       if (n < 0 || n >= (int)sizeof(tmpfile_path))
+       if (n + strlen(tmpname_tmpl) + 1 > sizeof(tmpfile_path))
                goto create_fail;
 
+       memcpy(tmpfile_path + n, tmpname_tmpl, strlen(tmpname_tmpl) + 1);
+
        fd = mkstemp(tmpfile_path);
        if (fd < 0)
                goto create_fail;

diff --git a/shared/util.c b/shared/util.c
index 2c6cfa96a73db9902b3e797c710218a603512202..fcb223508d7bde3de0e0ac946dd46d62654c2d96 100644 (file)
--- a/shared/util.c
+++ b/shared/util.c
@@ -476,10 +476,9 @@ int mkdir_parents(const char *path, mode_t mode)
        return mkdir_p(path, end - path, mode);
 }
 
-char *fd_lookup_path(int fd)
+int fd_lookup_path(int fd, char *path, size_t pathlen)
 {
        char proc_path[PATH_MAX];
-       char fd_path[PATH_MAX];
        ssize_t len;
 
        snprintf(proc_path, sizeof(proc_path), "/proc/self/fd/%d", fd);
@@ -488,13 +487,14 @@ char *fd_lookup_path(int fd)
         * We are using mkstemp to create a temporary file. We need to read the link since
         * the mkstemp creates file with an absolute path
         */
-       len = readlink(proc_path, fd_path, sizeof(fd_path) - 1);
-       if (len < 0 || len >= (ssize_t)sizeof(fd_path))
-               return NULL;
+       len = readlink(proc_path, path, pathlen - 1);
+       if (len < 0)
+               return (int)len;
 
-       fd_path[len] = '\0';
+       /* readlink(3), may null-terminate but should not be relied upon */
+       path[len] = '\0';
 
-       return strdup(fd_path);
+       return len;
 }
 
 static unsigned long long ts_usec(const struct timespec *ts)

diff --git a/shared/util.h b/shared/util.h
index c36d39e5392608491901786857c04991df628734..7363f4267eac58d7f1ddc88fc7adf850b4d01efb 100644 (file)
--- a/shared/util.h
+++ b/shared/util.h
@@ -54,7 +54,7 @@ static inline _must_check_ _nonnull_all_ bool path_is_absolute(const char *p)
 int mkdir_p(const char *path, int len, mode_t mode);
 int mkdir_parents(const char *path, mode_t mode);
 unsigned long long stat_mstamp(const struct stat *st);
-char *fd_lookup_path(int fd);
+_nonnull_all_ int fd_lookup_path(int fd, char *path, size_t pathlen);
 
 /* time-related functions
  * ************************************************************************ */