From 7f4096be412e6787dbf51453b2208a33ee56021d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Tue, 26 Aug 2025 18:30:46 +0200 Subject: [PATCH] Core: treat already-existing directory as 'ok' in recursive mkdir When creating a directory recursively (via `BLI_file_ensure_parent_dir_exists()` or `BLI_dir_create_recursive()`, filter out errors that indicate that the directory already exists. This is now also covered by a unit test. I've also removed `BLI_assert(BLI_exists(dirname) == 0);` from `dir_create_recursive()` (the workhorse for the above-mentioned functions), for two reasons: - The function is only used in an "ensure this directory exist" context, so the directory existing should be fine, and - the assertion doesn't guarantee that the subsequent call to `mkdir()` actually succeeds. Race conditions between Blender and other processes (potentially on other computers, when using networked filesystems) will make such a precondition test unreliable. This is a followup of 00abaa571a6319f5e092dbeb3edce9c08bd03102. Pull Request: https://projects.blender.org/blender/blender/pulls/145173 --- source/blender/blenlib/intern/fileops_c.cc | 27 +++++++++++++++++-- .../blender/blenlib/tests/BLI_fileops_test.cc | 18 +++++++++++++ 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/source/blender/blenlib/intern/fileops_c.cc b/source/blender/blenlib/intern/fileops_c.cc index ea7d5066909..bdf4f7fc66f 100644 --- a/source/blender/blenlib/intern/fileops_c.cc +++ b/source/blender/blenlib/intern/fileops_c.cc @@ -341,10 +341,19 @@ bool BLI_file_touch(const char *filepath) return false; } -static bool dir_create_recursive(char *dirname, int len) +/** + * Create the given directory and its parents if necessary. + * + * If the directory already exists, this function is a no-op. + * + * \param dirname The directory to create. + * \param len The number of bytes of 'dirname' to use as path to create. This + * makes the recursive call possible without doing string duplication for each + * parent directory. + */ +static bool dir_create_recursive(const char *dirname, const int len) { BLI_assert(strlen(dirname) == len); - BLI_assert(BLI_exists(dirname) == 0); /* Caller must ensure the path doesn't have trailing slashes. */ BLI_assert_msg(len && !BLI_path_slash_is_native_compat(dirname[len - 1]), "Paths must not end with a slash!"); @@ -375,12 +384,26 @@ static bool dir_create_recursive(char *dirname, int len) *dirname_parent_end = dirname_parent_end_value; } if (ret) { + /* Ignore errors when the directory was created (probably by another process) in between the + * earlier call to BLI_exists() and this call to mkdir. Since this function only creates a + * directory if it doesn't exist yet, this is actually not seen as an error, even though + * mkdir() failed. */ #ifdef WIN32 if (umkdir(dirname) == -1) { + if (GetLastError() == ERROR_ALREADY_EXISTS && BLI_is_dir(dirname)) { + return true; + } + + /* Any other error should bubble up as an actual error. */ ret = false; } #else if (mkdir(dirname, 0777) != 0) { + if (errno == EEXIST && BLI_is_dir(dirname)) { + return true; + } + + /* Any other error should bubble up as an actual error. */ ret = false; } #endif diff --git a/source/blender/blenlib/tests/BLI_fileops_test.cc b/source/blender/blenlib/tests/BLI_fileops_test.cc index 91d5dc28577..c2b56cb2472 100644 --- a/source/blender/blenlib/tests/BLI_fileops_test.cc +++ b/source/blender/blenlib/tests/BLI_fileops_test.cc @@ -157,6 +157,24 @@ TEST_F(FileOpsTest, rename) ASSERT_TRUE(BLI_exists(test_dirpath_dst.c_str())); } +TEST_F(FileOpsTest, dir_create_recursive) +{ + const std::string dir_path = this->temp_dir + SEP_STR + "dir-to-create"; + const std::string subdir_path = dir_path + SEP_STR + "subdir"; + + ASSERT_FALSE(BLI_exists(dir_path.c_str())); + ASSERT_TRUE(BLI_dir_create_recursive(subdir_path.c_str())); + ASSERT_TRUE(BLI_exists(subdir_path.c_str())); + + ASSERT_TRUE(BLI_dir_create_recursive(subdir_path.c_str())) + << "Creating an already-existing directory should be fine"; + + const std::string subfile_path = dir_path + SEP_STR + "some_file.txt"; + ASSERT_TRUE(BLI_file_touch(subfile_path.c_str())); + ASSERT_FALSE(BLI_dir_create_recursive(subfile_path.c_str())) + << "Creating a directory that already exists as file should return an error status"; +} + /* * blender::fstream tests. */