BLI_fileops: Harmonize 'rename' behaviors accross platforms.
This commit aim at making the behaviors of `BLI_rename` and `BLI_rename_overwrite` more consistent and coherent across all supported platforms. * `BLI_rename` now only succeeds in case the target `to` path does not exists (similar to Windows `rename` behavior). * `BLI_rename_overwrite` allows to replace an existing target `to` file or (empty) directory (similar to Unix `rename` behavior). NOTE: In case the target is open by some process on the system, trying to overwrite it will still fail on Windows, while it should succeed on Unix-like systems. The main change for Windows is the usage of `MoveFileExW` instead of `_wrename`, which allows for 'native support' of file overwrite (using the `MOVEFILE_REPLACE_EXISTING` flag). Directories still need to be explicitly removed though. The main change for *nix systems is the use of `renamex_np` (OSX) or `renameat2` (most Linux systems) to allow forbidding renaming to an already existing target in an 'atomic' way. NOTE: While this commit aims at avoiding the TOC/TOU problem as much as possible by using available system's primitives for most common cases, there are some situations where race conditions (filesystem changes between checks on FS state, and actual rename operation) remain possible. Pull Request: https://projects.blender.org/blender/blender/pulls/115096
This commit is contained in:
Bastien Montagne
Bastien Montagne
parent
dfe1a7d039
commit
050d48edfc
|
|
@ -72,14 +72,17 @@ int uaccess(const char *filename, int mode)
|
|||
return r;
|
||||
}
|
||||
|
||||
int urename(const char *oldname, const char *newname)
|
||||
int urename(const char *oldname, const char *newname, const bool do_replace)
|
||||
{
|
||||
int r = -1;
|
||||
UTF16_ENCODE(oldname);
|
||||
UTF16_ENCODE(newname);
|
||||
|
||||
if (oldname_16 && newname_16) {
|
||||
r = _wrename(oldname_16, newname_16);
|
||||
/* Closer to UNIX `rename` behavior, as it at least allows to replace an existing file.
|
||||
* Return value logic is inverted however (returns non-zero on sucess, 0 on failure).
|
||||
* Note that the operation will still fail if the 'newname' existing file is opened anywhere. */
|
||||
r = (MoveFileExW(oldname_16, newname_16, do_replace ? MOVEFILE_REPLACE_EXISTING : 0) == 0);
|
||||
}
|
||||
|
||||
UTF16_UN_ENCODE(newname);
|
||||
|
|
|
|||
|
|
@ -18,7 +18,7 @@
|
|||
FILE *ufopen(const char *filename, const char *mode);
|
||||
int uopen(const char *filename, int oflag, int pmode);
|
||||
int uaccess(const char *filename, int mode);
|
||||
int urename(const char *oldname, const char *newname);
|
||||
int urename(const char *oldname, const char *newname, const bool do_replace);
|
||||
|
||||
char *u_alloc_getenv(const char *varname);
|
||||
void u_free_getenv(char *val);
|
||||
|
|
|
|||
|
|
@ -52,18 +52,36 @@ int BLI_copy(const char *path_src, const char *path_dst) ATTR_NONNULL();
|
|||
int BLI_path_move(const char *path_src, const char *path_dst) ATTR_NONNULL();
|
||||
|
||||
/**
|
||||
* Rename a file or directory.
|
||||
* Rename a file or directory, unless `to` already exists.
|
||||
*
|
||||
* \note This matches Windows `rename` logic, _not_ Unix one. It does not allow to replace an
|
||||
* existing target. Use #BLI_rename_overwrite instead if existing file should be replaced.
|
||||
*
|
||||
* \param from: The path to rename from (return failure if it does not exist).
|
||||
* \param to: The destination path.
|
||||
* \return zero on success (matching 'rename' behavior).
|
||||
*/
|
||||
int BLI_rename(const char *from, const char *to);
|
||||
int BLI_rename(const char *from, const char *to) ATTR_NONNULL();
|
||||
|
||||
/**
|
||||
* Rename a file or directory.
|
||||
* Rename a file or directory, replacing target `to` path if it exists.
|
||||
*
|
||||
* \warning It's up to the caller to ensure `from` & `to` don't point to the same file
|
||||
* as this will result in `to` being deleted to make room for `from`
|
||||
* (which will then also be deleted).
|
||||
* \note This matches Unix `rename` logic. It does allow to replace an existing target. Use
|
||||
* #BLI_rename instead if existing file should never be replaced. However, if `to` is an existing,
|
||||
* non-empty directory, the operation will fail.
|
||||
*
|
||||
* \note There is still no feature-parity between behaviors on Windows and Unix, in case the target
|
||||
* `to` exists and is opened by some process in the system:
|
||||
* - On Unix, it will typically succeed
|
||||
* (see https://man7.org/linux/man-pages/man2/rename.2.html for details).
|
||||
* - On Windows, it will always fail
|
||||
* (see https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-movefileexw for
|
||||
* details).
|
||||
*
|
||||
* \warning Due to internal limitation/implementation, on Windows, in case paths point to
|
||||
* directories, it's up to the caller to ensure that `from` and `to` are not the same directory.
|
||||
* Since `to` is being deleted to make room for `from`, this will result in `from` being deleted as
|
||||
* well.
|
||||
*
|
||||
* See #BLI_path_move to move directories.
|
||||
*
|
||||
|
|
|
|||
|
|
@ -435,15 +435,6 @@ bool BLI_file_ensure_parent_dir_exists(const char *filepath)
|
|||
}
|
||||
|
||||
int BLI_rename(const char *from, const char *to)
|
||||
{
|
||||
#ifdef WIN32
|
||||
return urename(from, to);
|
||||
#else
|
||||
return rename(from, to);
|
||||
#endif
|
||||
}
|
||||
|
||||
int BLI_rename_overwrite(const char *from, const char *to)
|
||||
{
|
||||
if (!BLI_exists(from)) {
|
||||
return 1;
|
||||
|
|
@ -462,13 +453,60 @@ int BLI_rename_overwrite(const char *from, const char *to)
|
|||
* In this particular case we would not want to follow symbolic-links as well.
|
||||
* Since this functionality isn't required at the moment, leave this as-is.
|
||||
* Noting it as a potential improvement. */
|
||||
|
||||
/* NOTE: To avoid the concurrency 'time of check/time of use' (TOC/TOU) issue, this code attemps
|
||||
* to use available solutions for an 'atomic' (file-system wise) rename operation, instead of
|
||||
* first checking for an existing `to` target path, and then doing the rename operation if it
|
||||
* does not exists at the time of check.
|
||||
*
|
||||
* Windows (through `MoveFileExW`) by default does not allow replacing an existing path. It is
|
||||
* however not clear whether its API is exposed to the TOC/TOU issue or not.
|
||||
*
|
||||
* On Linux or OSX, to keep operations atomic, special non-standardized variants of `rename` must
|
||||
* be used, depending on the OS. Note that there may also be failure due to file system not
|
||||
* supporting this operation, although in practice this should not be a problem in modern
|
||||
* systems.
|
||||
* - https://man7.org/linux/man-pages/man2/rename.2.html
|
||||
* - https://www.unix.com/man-page/mojave/2/renameatx_np/
|
||||
*
|
||||
* BSD systems do not have any such thing currently, and are therefore exposed to the TOC/TOU
|
||||
* issue. */
|
||||
|
||||
#ifdef WIN32
|
||||
return urename(from, to, false);
|
||||
#elif defined(__APPLE__)
|
||||
return renamex_np(from, to, RENAME_EXCL);
|
||||
#elif defined(__GLIBC_PREREQ) && __GLIBC_PREREQ(2, 28)
|
||||
/* Most common Linux cases. */
|
||||
return renameat2(AT_FDCWD, from, AT_FDCWD, to, RENAME_NOREPLACE);
|
||||
#else
|
||||
/* At least all BSD's currently. */
|
||||
if (BLI_exists(to)) {
|
||||
if (BLI_delete(to, false, false)) {
|
||||
return 1;
|
||||
}
|
||||
return rename(from, to);
|
||||
#endif
|
||||
}
|
||||
|
||||
int BLI_rename_overwrite(const char *from, const char *to)
|
||||
{
|
||||
if (!BLI_exists(from)) {
|
||||
return 1;
|
||||
}
|
||||
|
||||
#ifdef WIN32
|
||||
/* `urename` from `utfconv` intern utils uses `MoveFileExW`, which allows to replace an existing
|
||||
* file, but not an existing directory, even if empty. This will only delete empty directories.
|
||||
*/
|
||||
if (BLI_is_dir(to)) {
|
||||
if (BLI_delete(to, true, false)) {
|
||||
return 1;
|
||||
}
|
||||
}
|
||||
|
||||
return BLI_rename(from, to);
|
||||
return urename(from, to, true);
|
||||
#else
|
||||
return rename(from, to);
|
||||
#endif
|
||||
}
|
||||
|
||||
#ifdef WIN32
|
||||
|
|
|
|||
|
|
@ -67,15 +67,9 @@ TEST_F(FileOpsTest, rename)
|
|||
BLI_file_touch(test_filepath_src.c_str());
|
||||
ASSERT_TRUE(BLI_exists(test_filepath_src.c_str()));
|
||||
|
||||
/* `test_filepath_dst` does exist now, so regular rename should succeed on Unix, but fail on
|
||||
* Windows. */
|
||||
#ifdef WIN32
|
||||
/* `test_filepath_dst` does exist now, so regular rename should fail. */
|
||||
ASSERT_NE(0, BLI_rename(test_filepath_src.c_str(), test_filepath_dst.c_str()));
|
||||
ASSERT_TRUE(BLI_exists(test_filepath_src.c_str()));
|
||||
#else
|
||||
ASSERT_EQ(0, BLI_rename(test_filepath_src.c_str(), test_filepath_dst.c_str()));
|
||||
ASSERT_FALSE(BLI_exists(test_filepath_src.c_str()));
|
||||
#endif
|
||||
ASSERT_TRUE(BLI_exists(test_filepath_dst.c_str()));
|
||||
|
||||
BLI_file_touch(test_filepath_src.c_str());
|
||||
|
|
@ -94,18 +88,6 @@ TEST_F(FileOpsTest, rename)
|
|||
*
|
||||
* This is expected to succeed on Unix, but fail on Windows. */
|
||||
int fd_dst = BLI_open(test_filepath_dst.c_str(), O_BINARY | O_RDONLY, 0);
|
||||
#ifdef WIN32
|
||||
ASSERT_NE(0, BLI_rename(test_filepath_src.c_str(), test_filepath_dst.c_str()));
|
||||
ASSERT_TRUE(BLI_exists(test_filepath_src.c_str()));
|
||||
#else
|
||||
ASSERT_EQ(0, BLI_rename(test_filepath_src.c_str(), test_filepath_dst.c_str()));
|
||||
ASSERT_FALSE(BLI_exists(test_filepath_src.c_str()));
|
||||
#endif
|
||||
ASSERT_TRUE(BLI_exists(test_filepath_dst.c_str()));
|
||||
|
||||
BLI_file_touch(test_filepath_src.c_str());
|
||||
ASSERT_TRUE(BLI_exists(test_filepath_src.c_str()));
|
||||
|
||||
#ifdef WIN32
|
||||
ASSERT_NE(0, BLI_rename_overwrite(test_filepath_src.c_str(), test_filepath_dst.c_str()));
|
||||
ASSERT_TRUE(BLI_exists(test_filepath_src.c_str()));
|
||||
|
|
@ -138,20 +120,17 @@ TEST_F(FileOpsTest, rename)
|
|||
BLI_dir_create_recursive(test_dirpath_src.c_str());
|
||||
ASSERT_TRUE(BLI_exists(test_dirpath_src.c_str()));
|
||||
|
||||
/* `test_dirpath_dst` now exists, so regular rename should succeed on Unix, but fail on Windows.
|
||||
*/
|
||||
#ifdef WIN32
|
||||
/* `test_dirpath_dst` now exists, so regular rename should fail. */
|
||||
ASSERT_NE(0, BLI_rename(test_dirpath_src.c_str(), test_dirpath_dst.c_str()));
|
||||
ASSERT_TRUE(BLI_exists(test_dirpath_src.c_str()));
|
||||
#else
|
||||
ASSERT_EQ(0, BLI_rename(test_dirpath_src.c_str(), test_dirpath_dst.c_str()));
|
||||
ASSERT_FALSE(BLI_exists(test_dirpath_src.c_str()));
|
||||
#endif
|
||||
ASSERT_TRUE(BLI_exists(test_dirpath_dst.c_str()));
|
||||
|
||||
#ifndef WIN32
|
||||
/* `test_dirpath_dst` now exists, but is empty, so overwrite rename should suceed. */
|
||||
ASSERT_EQ(0, BLI_rename_overwrite(test_dirpath_src.c_str(), test_dirpath_dst.c_str()));
|
||||
ASSERT_FALSE(BLI_exists(test_dirpath_src.c_str()));
|
||||
ASSERT_TRUE(BLI_exists(test_dirpath_dst.c_str()));
|
||||
|
||||
BLI_dir_create_recursive(test_dirpath_src.c_str());
|
||||
#endif
|
||||
ASSERT_TRUE(BLI_exists(test_dirpath_src.c_str()));
|
||||
|
||||
const std::string test_dir_filepath_src = test_dirpath_src + SEP_STR + file_name_src;
|
||||
|
|
@ -167,13 +146,12 @@ TEST_F(FileOpsTest, rename)
|
|||
ASSERT_FALSE(BLI_exists(test_dir_filepath_src.c_str()));
|
||||
ASSERT_TRUE(BLI_exists(test_dir_filepath_dst.c_str()));
|
||||
|
||||
/* `test_dirpath_dst` exists and is not empty, so regular rename should fail on all platforms. */
|
||||
/* `test_dirpath_dst` exists and is not empty, so regular rename should fail. */
|
||||
ASSERT_NE(0, BLI_rename(test_dirpath_src.c_str(), test_dirpath_dst.c_str()));
|
||||
ASSERT_TRUE(BLI_exists(test_dirpath_src.c_str()));
|
||||
ASSERT_TRUE(BLI_exists(test_dirpath_dst.c_str()));
|
||||
|
||||
/* `test_dirpath_dst` exists and is not empty, so even overwrite rename should fail on all
|
||||
* platforms. */
|
||||
/* `test_dirpath_dst` exists and is not empty, so even overwrite rename should fail. */
|
||||
ASSERT_NE(0, BLI_rename_overwrite(test_dirpath_src.c_str(), test_dirpath_dst.c_str()));
|
||||
ASSERT_TRUE(BLI_exists(test_dirpath_src.c_str()));
|
||||
ASSERT_TRUE(BLI_exists(test_dirpath_dst.c_str()));
|
||||
|
|
|
|||
Loading…
Reference in New Issue