commit: c562d8f046f22132621b49958ea9fe48db423962
parent 8aec98a517fdbbb0fc303e64ad5c7f61babdfd1b
Author: Haelwenn (lanodan) Monnier <contact@hacktivis.me>
Date: Sat, 24 Jan 2026 15:35:20 +0100
Revert "libutils/lib_mkdir: mkdir+chmod when making parents, check filetype after EEXIST"
As could be predicted, it ends up creating a concurrency issue
when multiple mkdir(1) invocations for neighboring subdirectories
are done at the same time.
This reverts commit 28b92a6bd493f0a2543ad6709f3a9f6f116ddb88.
As far as I can tell the recursion issue was caused by keeping
the trailing slash so can also be safely reverted.
This reverts commit 7c50cbab04faffa818b127dd3b55047dc01cfc4f.
Diffstat:
5 files changed, 25 insertions(+), 182 deletions(-)
diff --git a/Makefile b/Makefile
@@ -33,7 +33,7 @@ selfcheck-cmds: $(EXE) $(TEST_CMDS)
LDSTATIC="$(LDSTATIC)" ./check-cmds.sh
.PHONY: selfcheck-libs
-TEST_LIBS = test-libutils/t_mode test-libutils/t_strtodur test-libutils/t_symbolize_mode test-libutils/t_truncation test-lib/t_sha1 test-lib/t_sha256 test-lib/t_sha512 test-libutils/t_humanize test-libutils/t_strlcpy test-libutils/t_strip_lastelem
+TEST_LIBS = test-libutils/t_mode test-libutils/t_strtodur test-libutils/t_symbolize_mode test-libutils/t_truncation test-lib/t_sha1 test-lib/t_sha256 test-lib/t_sha512 test-libutils/t_humanize test-libutils/t_strlcpy
selfcheck-libs: $(TEST_LIBS)
LDSTATIC="$(LDSTATIC)" ./check-libs.sh $(TEST_LIBS)
diff --git a/libutils/lib_mkdir.c b/libutils/lib_mkdir.c
@@ -7,8 +7,6 @@
#include "./lib_mkdir.h"
#include "./lib_string.h"
-#include "./mode.h"
-#include "./strip_lastelem.h"
#include <errno.h>
#include <limits.h> // PATH_MAX
@@ -16,107 +14,46 @@
#include <string.h> // strlen, strerror
#include <sys/stat.h> // mkdir
-static int
-wrap_mkdir(const char *path, mode_t mode)
+int
+mkdir_parents(char *path, mode_t mode)
{
- if(mkdir(path, mode) < 0)
+ for(int i = strlen(path) - 1; i >= 0; i--)
{
- if(errno != EEXIST)
- {
- fprintf(
- stderr, "%s: error: Failed making directory '%s': %s\n", argv0, path, strerror(errno));
- errno = 0;
- return -1;
- }
-
- errno = 0;
-
- /*
- * As seen from GNU. I believe it shouldn't introduce a TOCTOU kind
- * of problem as it's a check which only invalidates.
- */
- struct stat dir_stat;
- if(stat(path, &dir_stat) < 0)
- {
- fprintf(stderr,
- "%s: error: Failed getting status of existing file or directory at '%s': %s\n",
- argv0,
- path,
- strerror(errno));
- return -1;
- }
+ if(path[i] != '/') break;
- if(!S_ISDIR(dir_stat.st_mode))
- {
- fprintf(stderr,
- "%s: error: Location '%s' already used by %s\n",
- argv0,
- path,
- filetype(&dir_stat));
- return -1;
- }
-
- errno = EEXIST;
- return 0;
+ path[i] = 0;
}
- else if(mkdir_parents_verbose)
- fprintf(stderr, "%s: Made directory: %s\n", argv0, path);
-
- return 0;
-}
-
-static int
-mkdir_elders(char *path, mode_t mode)
-{
- if(path[0] == '\0') return 0;
char parent[PATH_MAX] = "";
lib_strlcpy(parent, path, PATH_MAX);
- strip_lastelem(parent);
-
- if(mkdir_elders(parent, mode) < 0) return -1;
- errno = 0;
- if(wrap_mkdir(path, 0) < 0) return -1;
-
- /*
- * mkdir()+chmod() flow introduced with http://austingroupbugs.net/view.php?id=161
- * released in Technical Corrigendum 1 XCU/TC1-2008/0122
- *
- * Sadly the October 9, 2009 meeting https://www.opengroup.org/austin/docs/austin_467.txt
- * doesn't provides more details
- */
- if(errno != EEXIST && chmod(path, mode) < 0)
+ for(int i = strlen(parent) - 1; i >= 0; i--)
{
- char mode_buf[11] = "";
- symbolize_mode(mode, mode_buf);
- fprintf(stderr,
- "%s: error: Failed applying mode (%07o/%s) on '%s': %s\n",
- argv0,
- mode,
- mode_buf,
- path,
- strerror(errno));
- return -1;
- }
+ if(parent[i] == '/') break;
- return 0;
-}
+ parent[i] = 0;
+ }
-int
-mkdir_parents(char *path, mode_t mode)
-{
if(path[0] == 0) return 0;
- char parent[PATH_MAX] = "";
- lib_strlcpy(parent, path, PATH_MAX);
- strip_lastelem(parent);
+ mode_t parent_mode = (S_IWUSR | S_IXUSR | ~mkdir_parents_filemask) & 0777;
+
+ if(mkdir_parents(parent, parent_mode) < 0) return -1;
- mode_t elders_mode = (S_IWUSR | S_IXUSR | ~mkdir_parents_filemask) & 0777;
+ if(mkdir(path, mode) < 0)
+ {
+ if(errno == EEXIST)
+ {
+ errno = 0;
+ return 0;
+ }
- if(mkdir_elders(parent, elders_mode) < 0) return -1;
+ fprintf(stderr, "%s: error: Failed making directory '%s': %s\n", argv0, path, strerror(errno));
+ errno = 0;
+ return -1;
+ }
- if(wrap_mkdir(path, mode) < 0) return -1;
+ if(mkdir_parents_verbose) fprintf(stderr, "%s: Made directory: %s\n", argv0, path);
return 0;
}
diff --git a/libutils/strip_lastelem.c b/libutils/strip_lastelem.c
@@ -1,28 +0,0 @@
-// utils-std: Collection of commonly available Unix tools
-// SPDX-FileCopyrightText: 2017 Haelwenn (lanodan) Monnier <contact+utils@hacktivis.me>
-// SPDX-License-Identifier: MPL-2.0
-
-#include "./strip_lastelem.h"
-
-#include <string.h>
-
-// "/foo/bar/" -> "/foo/"
-// "/foo/bar" -> "/foo/"
-// "/" -> ""
-void
-strip_lastelem(char *parent)
-{
- size_t i = strlen(parent);
- if(i == 0)
- return;
- else
- i--;
-
- while(i > 0 && parent[i] == '/')
- parent[i--] = '\0';
-
- while(i > 0 && parent[i] != '/')
- parent[i--] = '\0';
-
- if(i == 0) parent[i--] = '\0';
-}
diff --git a/libutils/strip_lastelem.h b/libutils/strip_lastelem.h
@@ -1,5 +0,0 @@
-// utils-std: Collection of commonly available Unix tools
-// SPDX-FileCopyrightText: 2017 Haelwenn (lanodan) Monnier <contact+utils@hacktivis.me>
-// SPDX-License-Identifier: MPL-2.0
-
-void strip_lastelem(char *parent);
diff --git a/test-libutils/t_strip_lastelem.c b/test-libutils/t_strip_lastelem.c
@@ -1,61 +0,0 @@
-// utils-std: Collection of commonly available Unix tools
-// SPDX-FileCopyrightText: 2017 Haelwenn (lanodan) Monnier <contact+utils@hacktivis.me>
-// SPDX-License-Identifier: MPL-2.0
-
-#define _POSIX_C_SOURCE 200809L
-#include "../libutils/strip_lastelem.h"
-
-#include <assert.h>
-#include <stdio.h> // printf
-#include <string.h> // strcmp
-
-int counter = 0;
-int err = 0;
-
-static void
-t_strip_lastelem(const char *in, const char *exp)
-{
- int id = ++counter;
- static char buf[512];
-
- strcpy(buf, in);
-
- strip_lastelem(buf);
-
- if(strcmp(buf, exp) == 0)
- {
- printf("ok %d - \"%s\" -> \"%s\"\n", id, in, exp);
- return;
- }
-
- err = 1;
- printf("not ok %d - \"%s\" -> \"%s\"\n", id, in, exp);
- printf("# Got: \"%s\"\n", buf);
-}
-
-int
-main(void)
-{
- int plan = 11;
- printf("1..%d\n", plan);
-
- t_strip_lastelem("", "");
-
- t_strip_lastelem("/", "");
- t_strip_lastelem("//", "");
-
- t_strip_lastelem("/foo", "");
- t_strip_lastelem("/foo/", "");
-
- t_strip_lastelem("/foo/bar", "/foo/");
- t_strip_lastelem("/foo/bar/", "/foo/");
-
- t_strip_lastelem("a", "");
- t_strip_lastelem("a/", "");
-
- t_strip_lastelem("a/b", "a/");
- t_strip_lastelem("a/b/", "a/");
-
- assert(counter == plan);
- return err;
-}
