tmpfile: port tempfn_*() to path_extract_*()

diff --git a/src/basic/tmpfile-util.c b/src/basic/tmpfile-util.c
index bac5eb6b26a0232acd9800118be76ee628ecf11f..5ee71d015887aa8441e57498bb13aa19a77945ff 100644 (file)
--- a/src/basic/tmpfile-util.c
+++ b/src/basic/tmpfile-util.c
@@ -94,16 +94,11 @@ int fmkostemp_safe(char *pattern, const char *mode, FILE **ret_f) {
 }
 
 int tempfn_xxxxxx(const char *p, const char *extra, char **ret) {
-        const char *fn;
-        char *t;
+        _cleanup_free_ char *d = NULL, *fn = NULL, *nf = NULL;
+        int r;
 
         assert(ret);
 
-        if (isempty(p))
-                return -EINVAL;
-        if (path_equal(p, "/"))
-                return -EINVAL;
-
         /*
          * Turns this:
          *         /foo/bar/waldo
@@ -112,34 +107,41 @@ int tempfn_xxxxxx(const char *p, const char *extra, char **ret) {
          *         /foo/bar/.#<extra>waldoXXXXXX
          */
 
-        fn = basename(p);
-        if (!filename_is_valid(fn))
-                return -EINVAL;
+        r = path_extract_directory(p, &d);
+        if (r < 0 && r != -EDESTADDRREQ) /* EDESTADDRREQ → No directory specified, just a filename */
+                return r;
 
-        extra = strempty(extra);
+        r = path_extract_filename(p, &fn);
+        if (r < 0)
+                return r;
 
-        t = new(char, strlen(p) + 2 + strlen(extra) + 6 + 1);
-        if (!t)
+        nf = strjoin(".#", strempty(extra), fn, "XXXXXX");
+        if (!nf)
                 return -ENOMEM;
 
-        strcpy(stpcpy(stpcpy(stpcpy(mempcpy(t, p, fn - p), ".#"), extra), fn), "XXXXXX");
+        if (!filename_is_valid(nf)) /* New name is not valid? (Maybe because too long?) Refuse. */
+                return -EINVAL;
+
+        if (d)  {
+                char *j;
+
+                j = path_join(d, nf);
+                if (!j)
+                        return -ENOMEM;
+
+                *ret = path_simplify(j, false);
+        } else
+                *ret = TAKE_PTR(nf);
 
-        *ret = path_simplify(t, false);
         return 0;
 }
 
 int tempfn_random(const char *p, const char *extra, char **ret) {
-        const char *fn;
-        char *t, *x;
-        uint64_t u;
+        _cleanup_free_ char *d = NULL, *fn = NULL, *nf = NULL;
+        int r;
 
         assert(ret);
 
-        if (isempty(p))
-                return -EINVAL;
-        if (path_equal(p, "/"))
-                return -EINVAL;
-
         /*
          * Turns this:
          *         /foo/bar/waldo
@@ -148,27 +150,34 @@ int tempfn_random(const char *p, const char *extra, char **ret) {
          *         /foo/bar/.#<extra>waldobaa2a261115984a9
          */
 
-        fn = basename(p);
-        if (!filename_is_valid(fn))
-                return -EINVAL;
+        r = path_extract_directory(p, &d);
+        if (r < 0 && r != -EDESTADDRREQ) /* EDESTADDRREQ → No directory specified, just a filename */
+                return r;
 
-        extra = strempty(extra);
+        r = path_extract_filename(p, &fn);
+        if (r < 0)
+                return r;
 
-        t = new(char, strlen(p) + 2 + strlen(extra) + 16 + 1);
-        if (!t)
+        if (asprintf(&nf, ".#%s%s%016" PRIx64,
+                     strempty(extra),
+                     fn,
+                     random_u64()) < 0)
                 return -ENOMEM;
 
-        x = stpcpy(stpcpy(stpcpy(mempcpy(t, p, fn - p), ".#"), extra), fn);
+        if (!filename_is_valid(nf)) /* Not valid? (maybe because too long now?) — refuse early */
+                return -EINVAL;
 
-        u = random_u64();
-        for (unsigned i = 0; i < 16; i++) {
-                *(x++) = hexchar(u & 0xF);
-                u >>= 4;
-        }
+        if (d) {
+                char *j;
 
-        *x = 0;
+                j = path_join(d, nf);
+                if (!j)
+                        return -ENOMEM;
+
+                *ret = path_simplify(j, false);
+        } else
+                *ret = TAKE_PTR(nf);
 
-        *ret = path_simplify(t, false);
         return 0;
 }
 

diff --git a/src/test/meson.build b/src/test/meson.build
index c567a4cfcbb63911f0ddccf93aabb558d58e61af..62e31fd830993f60ef56a8ecffb9ef739e698b8e 100644 (file)
--- a/src/test/meson.build
+++ b/src/test/meson.build
@@ -379,6 +379,8 @@ tests += [
 
         [['src/test/test-clock.c']],
 
+        [['src/test/test-tmpfile-util.c']],
+
         [['src/test/test-architecture.c']],
 
         [['src/test/test-log.c']],

diff --git a/src/test/test-tmpfile-util.c b/src/test/test-tmpfile-util.c
new file mode 100644 (file)
index 0000000..83bac15
--- /dev/null
+++ b/src/test/test-tmpfile-util.c
@@ -0,0 +1,102 @@
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+
+#include "alloc-util.h"
+#include "errno-util.h"
+#include "log.h"
+#include "string-util.h"
+#include "tests.h"
+#include "tmpfile-util.h"
+
+static void test_tempfn_random_one(const char *p, const char *extra, const char *expect, int ret) {
+        _cleanup_free_ char *s = NULL;
+        int r;
+
+        r = tempfn_random(p, extra, &s);
+        log_info_errno(r, "%s+%s → %s vs. %s (%i/%m vs. %i/%s)", p, strna(extra), strna(s), strna(expect), r, ret, strerror_safe(ret));
+
+        assert(!s == !expect);
+        if (s) {
+                const char *suffix;
+
+                assert_se(suffix = startswith(s, expect));
+                assert_se(in_charset(suffix, HEXDIGITS));
+                assert_se(strlen(suffix) == 16);
+        }
+        assert(ret == r);
+}
+
+static void test_tempfn_random(void) {
+        test_tempfn_random_one("", NULL, NULL, -EINVAL);
+        test_tempfn_random_one(".", NULL, NULL, -EINVAL);
+        test_tempfn_random_one("..", NULL, NULL, -EINVAL);
+        test_tempfn_random_one("/", NULL, NULL, -EADDRNOTAVAIL);
+
+        test_tempfn_random_one("foo", NULL, ".#foo", 0);
+        test_tempfn_random_one("foo", "bar", ".#barfoo", 0);
+        test_tempfn_random_one("/tmp/foo", NULL, "/tmp/.#foo", 0);
+        test_tempfn_random_one("/tmp/foo", "bar", "/tmp/.#barfoo", 0);
+        test_tempfn_random_one("./foo", NULL, "./.#foo", 0);
+        test_tempfn_random_one("./foo", "bar", "./.#barfoo", 0);
+        test_tempfn_random_one("../foo", NULL, "../.#foo", 0);
+        test_tempfn_random_one("../foo", "bar", "../.#barfoo", 0);
+
+        test_tempfn_random_one("foo/", NULL, ".#foo", 0);
+        test_tempfn_random_one("foo/", "bar", ".#barfoo", 0);
+        test_tempfn_random_one("/tmp/foo/", NULL, "/tmp/.#foo", 0);
+        test_tempfn_random_one("/tmp/foo/", "bar", "/tmp/.#barfoo", 0);
+        test_tempfn_random_one("./foo/", NULL, "./.#foo", 0);
+        test_tempfn_random_one("./foo/", "bar", "./.#barfoo", 0);
+        test_tempfn_random_one("../foo/", NULL, "../.#foo", 0);
+        test_tempfn_random_one("../foo/", "bar", "../.#barfoo", 0);
+}
+
+static void test_tempfn_xxxxxx_one(const char *p, const char *extra, const char *expect, int ret) {
+        _cleanup_free_ char *s = NULL;
+        int r;
+
+        r = tempfn_xxxxxx(p, extra, &s);
+        log_info_errno(r, "%s+%s → %s vs. %s (%i/%m vs. %i/%s)", p, strna(extra), strna(s), strna(expect), r, ret, strerror_safe(ret));
+
+        assert(!s == !expect);
+        if (s) {
+                const char *suffix;
+
+                assert_se(suffix = startswith(s, expect));
+                assert_se(streq(suffix, "XXXXXX"));
+        }
+        assert(ret == r);
+}
+
+static void test_tempfn_xxxxxx(void) {
+        test_tempfn_xxxxxx_one("", NULL, NULL, -EINVAL);
+        test_tempfn_xxxxxx_one(".", NULL, NULL, -EINVAL);
+        test_tempfn_xxxxxx_one("..", NULL, NULL, -EINVAL);
+        test_tempfn_xxxxxx_one("/", NULL, NULL, -EADDRNOTAVAIL);
+
+        test_tempfn_xxxxxx_one("foo", NULL, ".#foo", 0);
+        test_tempfn_xxxxxx_one("foo", "bar", ".#barfoo", 0);
+        test_tempfn_xxxxxx_one("/tmp/foo", NULL, "/tmp/.#foo", 0);
+        test_tempfn_xxxxxx_one("/tmp/foo", "bar", "/tmp/.#barfoo", 0);
+        test_tempfn_xxxxxx_one("./foo", NULL, "./.#foo", 0);
+        test_tempfn_xxxxxx_one("./foo", "bar", "./.#barfoo", 0);
+        test_tempfn_xxxxxx_one("../foo", NULL, "../.#foo", 0);
+        test_tempfn_xxxxxx_one("../foo", "bar", "../.#barfoo", 0);
+
+        test_tempfn_xxxxxx_one("foo/", NULL, ".#foo", 0);
+        test_tempfn_xxxxxx_one("foo/", "bar", ".#barfoo", 0);
+        test_tempfn_xxxxxx_one("/tmp/foo/", NULL, "/tmp/.#foo", 0);
+        test_tempfn_xxxxxx_one("/tmp/foo/", "bar", "/tmp/.#barfoo", 0);
+        test_tempfn_xxxxxx_one("./foo/", NULL, "./.#foo", 0);
+        test_tempfn_xxxxxx_one("./foo/", "bar", "./.#barfoo", 0);
+        test_tempfn_xxxxxx_one("../foo/", NULL, "../.#foo", 0);
+        test_tempfn_xxxxxx_one("../foo/", "bar", "../.#barfoo", 0);
+}
+
+int main(int argc, char **argv) {
+        test_setup_logging(LOG_DEBUG);
+
+        test_tempfn_random();
+        test_tempfn_xxxxxx();
+
+        return 0;
+}