Remove mkdir_p()

It induced too many redundant mkdir() and stat() system calls.

It's now best to use mkdir() directly, since the cache structure no
longer involves long and pointless directory chains.

Incidentally fixes a missing mkdir("cache/fallback"), which was
preventing the cache from backing up any valid RPPs.

diff --git a/src/cache.c b/src/cache.c
index bf89c17efaf9c07e4efece2b13f98e2b91567e45..07757c733e83529b7865790d6f04a7e9cf1e9806 100644 (file)
--- a/src/cache.c
+++ b/src/cache.c
@@ -235,18 +235,22 @@ init_cachedir_tag(void)
 }
 
 static int
-init_tmp_dir(void)
+init_cache_dirs(void)
 {
        int error;
-
-       if (mkdir(CACHE_TMPDIR, CACHE_FILEMODE) < 0) {
-               error = errno;
-               if (error != EEXIST)
-                       return pr_op_err("Cannot create '%s': %s",
-                           CACHE_TMPDIR, strerror(error));
-       }
-
-       return 0;
+       error = file_mkdir("rsync", true);
+       if (error)
+               return error;
+       error = file_mkdir("https", true);
+       if (error)
+               return error;
+       error = file_mkdir("rrdp", true);
+       if (error)
+               return error;
+       error = file_mkdir("fallback", true);
+       if (error)
+               return error;
+       return file_mkdir(CACHE_TMPDIR, true);
 }
 
 int
@@ -261,7 +265,7 @@ cache_setup(void)
        // XXX Lock the cache directory
        init_tables();
        init_cache_metafile();
-       init_tmp_dir();
+       init_cache_dirs();
        init_cachedir_tag();
        return 0;
 }
@@ -975,6 +979,7 @@ commit_fallbacks(void)
        struct cache_commit *commit;
        struct cache_node *fb, *tmp;
        array_index i;
+       int error;
 
        while (!STAILQ_EMPTY(&commits)) {
                commit = STAILQ_FIRST(&commits);
@@ -983,13 +988,8 @@ commit_fallbacks(void)
                if (commit->caRepository) {
                        fb = provide_node(&cache.fallback, commit->caRepository);
 
-                       if (mkdir(fb->map.path, CACHE_FILEMODE) < 0) {
-                               if (errno != EEXIST) {
-                                       pr_op_warn("Failed to create %s: %s",
-                                           fb->map.path, strerror(errno));
-                                       goto skip;
-                               }
-                       }
+                       if (file_mkdir(fb->map.path, true) != 0)
+                               goto skip;
 
                        commit_rpp(commit, fb);
                        discard_trash(commit, fb);
@@ -1027,9 +1027,10 @@ skip:            free(commit->caRepository);
                 * from an expiration threshold.
                 */
                pr_op_debug("Removing orphaned fallback: %s", fb->map.path);
-               if (file_rm_rf(fb->map.path) < 0)
-                       pr_op_warn("Could not remove %s; unknown cause.",
-                           fb->map.path);
+               error = file_rm_rf(fb->map.path);
+               if (error)
+                       pr_op_warn("%s removal failed: %s",
+                           fb->map.path, strerror(error));
                delete_node(&cache.fallback, fb);
        }
 }

diff --git a/src/common.c b/src/common.c
index e73ddc03634eceda796178d1ae70ec96d9cee354..1f7d7e6809a2b4282cdca3559fa4a0da83cb7673 100644 (file)
--- a/src/common.c
+++ b/src/common.c
@@ -238,83 +238,6 @@ valid_file_or_dir(char const *location, bool check_file)
        return result;
 }
 
-static int
-stat_dir(char const *path)
-{
-       struct stat meta;
-
-       if (stat(path, &meta) != 0)
-               return errno;
-       if (!S_ISDIR(meta.st_mode))
-               return ENOTDIR;
-       return 0;
-}
-
-static int
-ensure_dir(char const *path)
-{
-       int error;
-
-       /*
-        * Perplexingly, mkdir() returns EEXIST instead of ENOTDIR when the
-        * path actually refers to a file.
-        * So it looks like this stat() is unavoidable.
-        */
-       if (stat_dir(path) == ENOTDIR && remove(path) < 0)
-               return errno;
-
-       if (mkdir(path, CACHE_FILEMODE) < 0) {
-               error = errno;
-               return (error == EEXIST) ? 0 : error;
-       }
-
-       return 0;
-}
-
-/* mkdir -p $_path */
-/* XXX Maybe also short-circuit by parent? */
-int
-mkdir_p(char const *_path, bool include_basename)
-{
-       char *path, *last_slash;
-       int i, result = 0;
-
-       path = pstrdup(_path); /* Remove const */
-
-       if (!include_basename) {
-               last_slash = strrchr(path, '/');
-               if (last_slash == NULL)
-                       goto end;
-               *last_slash = '\0';
-       }
-
-       result = stat_dir(path); /* short circuit */
-       if (result == 0)
-               goto end; /* Already exists */
-       if (result != ENOENT && result != ENOTDIR) {
-               pr_op_err_st("Error during stat(%s): %s",
-                   path, strerror(result));
-               goto end;
-       }
-
-       for (i = 1; path[i] != '\0'; i++) {
-               if (path[i] == '/') {
-                       path[i] = '\0';
-                       result = ensure_dir(path);
-                       path[i] = '/';
-                       if (result != 0) {
-                               pr_op_err_st("Error during mkdir(%s): %s",
-                                   path, strerror(result));
-                               goto end;
-                       }
-               }
-       }
-       result = ensure_dir(path);
-
-end:   free(path);
-       return result;
-}
-
 time_t
 time_nonfatal(void)
 {

diff --git a/src/common.h b/src/common.h
index 3bc5c1d8226f62601358cca572c1820e98dc2bfc..63c37b615f917eaffc547b7cf97d7aa93ca7848c 100644 (file)
--- a/src/common.h
+++ b/src/common.h
@@ -47,8 +47,6 @@ int foreach_file(char const *, char const *, bool, foreach_file_cb, void *);
 // XXX
 bool valid_file_or_dir(char const *, bool);
 
-int mkdir_p(char const *, bool);
-
 time_t time_nonfatal(void);
 time_t time_fatal(void);
 

diff --git a/src/config.c b/src/config.c
index bcfe64b1eeaf01b199332393d42ec29c5d06fdf7..2957c67569fb2d22ddf136597ba901d7e453d38a 100644 (file)
--- a/src/config.c
+++ b/src/config.c
@@ -1211,7 +1211,7 @@ handle_flags_config(int argc, char **argv)
        }
 
        if (rpki_config.daemon) {
-               pr_op_warn("Executing as daemon, all logs will be sent to syslog.");
+               pr_op_info("Executing as daemon, all logs will be sent to syslog.");
                /* Send all logs to syslog */
                rpki_config.log.output = SYSLOG;
                rpki_config.validation_log.output = SYSLOG;

diff --git a/src/file.c b/src/file.c
index e256f79b0bc625639403b0d1da13ed211512fdf5..5647053945f8bae0afe82578d43af12f01abb43c 100644 (file)
--- a/src/file.c
+++ b/src/file.c
@@ -50,8 +50,8 @@ file_write(char const *file_name, char const *mode, FILE **result)
        file = fopen(file_name, mode);
        if (file == NULL) {
                error = errno;
-               pr_val_err("Could not open file '%s': %s", file_name,
-                   strerror(error));
+               pr_val_err("Could not open file '%s' for writing: %s",
+                   file_name, strerror(error));
                *result = NULL;
                return error;
        }
@@ -70,10 +70,6 @@ file_write_full(char const *path, unsigned char const *content,
 
        pr_val_debug("Writing file: %s", path);
 
-       error = mkdir_p(path, false);
-       if (error)
-               return error;
-
        error = file_write(path, "wb", &out);
        if (error)
                return error;
@@ -167,83 +163,6 @@ file_exists(char const *path)
        return (stat(path, &meta) == 0) ? 0 : errno;
 }
 
-/* strlen("cache/tmp/123"), ie. 13 */
-static size_t src_offset;
-/* cache/rsync/a.b.c/d/e */
-static char const *merge_dst;
-
-/* Moves cache/tmp/123/z into cache/rsync/a.b.c/d/e/z. */
-static int
-merge_into(const char *src, const struct stat *st, int typeflag,
-    struct FTW *ftw)
-{
-       char *dst;
-       struct timespec times[2];
-
-       dst = path_join(merge_dst, &src[src_offset]);
-
-       if (S_ISDIR(st->st_mode)) {
-               pr_op_debug("mkdir -p %s", dst);
-               if (mkdir_p(dst, true)) {
-                       PR_DEBUG_MSG("Failed: %s", strerror(errno));
-                       goto end;
-               }
-
-               times[0] = st->st_atim;
-               times[1] = st->st_mtim;
-               if (utimensat(AT_FDCWD, dst, times, AT_SYMLINK_NOFOLLOW) < 0)
-                       PR_DEBUG_MSG("utimensat: %s", strerror(errno));
-       } else {
-               pr_op_debug("rename: %s -> %s", src, dst);
-               if (rename(src, dst) < 0) {
-                       if (errno == EISDIR) {
-                               /* XXX stacked nftw()s */
-                               if (file_rm_rf(dst) != 0)
-                                       PR_DEBUG_MSG("%s", "AAAAAAAAAAA");
-                               if (rename(src, dst) < 0)
-                                       PR_DEBUG_MSG("rename: %s", strerror(errno));
-                       }
-               }
-       }
-
-end:   free(dst);
-       return 0;
-}
-
-/*
- * Move all the files contained in @src to @dst, overwriting when necessary,
- * not touching files that exist in @dst but not in @src.
- *
- * @src must exist.
- *
- * @src: cache/tmp/123
- * @dst: cache/rsync/a.b.c/d/e
- */
-int
-file_merge_into(char const *src, char const *dst)
-{
-       int error;
-
-       error = mkdir_p(dst, false);
-       if (error)
-               return error;
-
-       if (file_exists(dst) == ENOENT) {
-               if (rename(src, dst) < 0) {
-                       error = errno;
-                       pr_op_err("Could not move %s to %s: %s",
-                           src, dst, strerror(error));
-                       return error;
-               }
-               return 0;
-       }
-
-       src_offset = strlen(src);
-       merge_dst = dst;
-       /* TODO (performance) optimize that 32 */
-       return nftw(src, merge_into, 32, FTW_PHYS);
-}
-
 /*
  * Like remove(), but don't care if the file is already deleted.
  */
@@ -265,15 +184,51 @@ static int
 rm(const char *fpath, const struct stat *sb, int typeflag, struct FTW *ftwbuf)
 {
        pr_op_debug("Deleting %s.", fpath);
-       return (remove(fpath) < 0) ? errno : 0;
+       if (remove(fpath) < 0)
+               pr_op_warn("Cannot delete %s: %s", fpath, strerror(errno));
+       return 0;
 }
 
 /* Same as `system("rm -rf <path>")`, but more portable and maaaaybe faster. */
 int
 file_rm_rf(char const *path)
 {
+       int error;
+
        /* TODO (performance) optimize that 32 */
-       return nftw(path, rm, 32, FTW_DEPTH | FTW_PHYS);
+       errno = 0;
+       switch (nftw(path, rm, 32, FTW_DEPTH | FTW_PHYS)) {
+       case 0:
+               return 0; /* Happy path */
+       case -1:
+               /*
+                * POSIX requires nftw() to set errno,
+                * but the Linux man page doesn't mention it at all...
+                */
+               error = errno;
+               return error ? error : -1;
+       }
+
+       /* This is supposed to be unreachable, but let's not panic. */
+       return -1;
+}
+
+/* If @force, don't treat EEXIST as an error. */
+int
+file_mkdir(char const *path, bool force)
+{
+       int error;
+
+       if (mkdir(path, CACHE_FILEMODE) < 0) {
+               error = errno;
+               if (!force || error != EEXIST) {
+                       pr_op_err("Cannot create '%s': %s",
+                           path, strerror(error));
+                       return error;
+               }
+       }
+
+       return 0;
 }
 
 void

diff --git a/src/file.h b/src/file.h
index eca2a3182ca397d2404a3267345cf45f80e0a483..3e05f1e69de968a1d1abb41b7e1855bd7498e08a 100644 (file)
--- a/src/file.h
+++ b/src/file.h
@@ -36,6 +36,7 @@ int file_exists(char const *);
 int file_merge_into(char const *, char const *);
 int file_rm_f(char const *);
 int file_rm_rf(char const *);
+int file_mkdir(char const *, bool);
 
 struct cache_sequence {
        char *prefix;

diff --git a/src/http.c b/src/http.c
index ef7d32756c28f18ae6463abdb3bf13f0b518b4bb..9b716929c4570d1878c6ce080d1ccebe1cbd4e55 100644 (file)
--- a/src/http.c
+++ b/src/http.c
@@ -388,11 +388,6 @@ http_download(char const *url, char const *path, curl_off_t ims, bool *changed)
 
        pr_val_info("HTTP GET: %s -> %s", url, path);
 
-       /* XXX might not be needed anymore */
-       error = mkdir_p(path, false);
-       if (error)
-               return error;
-
        for (r = 0; true; r++) {
                pr_val_debug("Download attempt #%u...", r + 1);
 

diff --git a/src/rrdp.c b/src/rrdp.c
index 1fcd6fe59f64ddc624bb2099925fa66cb0374cac..30ff269d7485f4d3d3a64ed70b723851a18ff130 100644 (file)
--- a/src/rrdp.c
+++ b/src/rrdp.c
@@ -1207,6 +1207,12 @@ rrdp_update(struct cache_mapping const *notif, time_t mtim, bool *changed,
                cseq_init(&old->seq, cage, true);
                STAILQ_INIT(&old->delta_hashes);
 
+               error = file_mkdir(cage, false);
+               if (error) {
+                       rrdp_state_free(old);
+                       goto clean_notif;
+               }
+
                error = handle_snapshot(&new, old);
                if (error) {
                        rrdp_state_free(old);

diff --git a/src/rsync.c b/src/rsync.c
index 6141f9b8fc26b323b9752d029ff2832dfc2dfa14..76e79ec96dde7cb7892abcbd00b8071455b738e0 100644 (file)
--- a/src/rsync.c
+++ b/src/rsync.c
@@ -295,10 +295,6 @@ rsync_download(char const *url, char const *path)
                        pr_val_debug("    %s", args[i]);
        }
 
-       error = mkdir_p(path, true);
-       if (error)
-               return error;
-
        retries = 0;
        do {
                child_status = 0;

diff --git a/test/cache_test.c b/test/cache_test.c
index b2ccd57c9acf393c00f930018e5dd2388e28ef35..c958ac9fb59690d831e5892a8b014378ba85e3f9 100644 (file)
--- a/test/cache_test.c
+++ b/test/cache_test.c
@@ -41,7 +41,7 @@ rsync_download(char const *url, char const *path)
        if (dl_error)
                return dl_error;
 
-       ck_assert_int_eq(0, mkdir_p(path, true));
+       ck_assert_int_eq(0, mkdir(path, CACHE_FILEMODE));
        touch_file(path);
 
        return 0;

diff --git a/test/rrdp_update_test.c b/test/rrdp_update_test.c
index 902c5dbd82d930d37c5cc51b1cdbd5501954c5c7..d4637712b8061ad6d48910fc545d1d9d4e1e6a66 100644 (file)
--- a/test/rrdp_update_test.c
+++ b/test/rrdp_update_test.c
@@ -22,7 +22,7 @@ static void
 setup_test(void)
 {
        ck_assert_int_eq(0, system("rm -rf tmp/"));
-       ck_assert_int_eq(0, system("mkdir -p tmp/rsync tmp/https tmp/tmp"));
+       ck_assert_int_eq(0, system("mkdir -p tmp/https tmp/rrdp tmp/tmp"));
        ck_assert_int_eq(0, hash_setup());
        ck_assert_int_eq(0, relax_ng_init());
 }