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 00abaa571a.
Pull Request: https://projects.blender.org/blender/blender/pulls/145173
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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.
|
||||
*/
|
||||
|
||||
Reference in New Issue
Block a user