feat(fatfs): add option to make rename() replace the destination

POSIX rename() silently replaces the destination when it already exists,
while FatFs' f_rename() refuses with FR_EXIST. rename() on a FAT mount
therefore fails with EEXIST where the same call succeeds on other file
systems, and every caller that wants portable behaviour has to remove the
destination itself.

Add CONFIG_FATFS_VFS_RENAME_REPLACES_DESTINATION (default n, so existing
behaviour is unchanged) to emulate the POSIX semantics: when f_rename()
reports FR_EXIST, remove the destination and retry, all under the lock
already held for the rename so no other VFS caller observes the gap.

The POSIX rules on what may replace what are applied before anything is
removed, because f_unlink() deletes empty directories as readily as
files and would otherwise discard a directory to make way for a file:
renaming a file onto a directory fails with EISDIR, a directory onto a
file with ENOTDIR, and a directory onto a non-empty directory with
ENOTEMPTY. Without the option all of these keep failing with EEXIST.

Moving a directory into its own subtree is rejected with EINVAL as well.
f_rename() does not check for this and links the directory into its own
tree, losing its contents, so this is a correctness fix rather than a
matter of which error is reported.

Renaming an entry to itself needs no special handling: f_rename() reports
FR_EXIST only when the destination resolves to a different directory
entry, so a self-rename, including one written with a different spelling
of the same name, already succeeds.

The emulation cannot be atomic, nor can it honour the POSIX guarantee
that a failed rename leaves an instance of the destination in place: FAT
cannot replace a directory entry in one step, so an interruption between
removing the destination and completing the rename can leave neither
name. This is documented in the option's help text.
This commit is contained in:
Tomáš Rohlínek
2026-09-03 09:44:24 +02:00
committed by BOT
parent bbf1dc9cc3
commit 4262553c56
5 changed files with 335 additions and 0 deletions

View File

@@ -321,6 +321,38 @@ menu "FAT Filesystem support"
This ensures that the link operation is atomic, but may cause performance for large files.
It may create less fragmented file copy.
config FATFS_VFS_RENAME_REPLACES_DESTINATION
bool "Make rename() replace an existing destination (POSIX behavior)"
default n
help
POSIX rename() silently replaces the destination if it already exists,
while FatFs' f_rename() refuses with FR_EXIST, so by default rename() on a
FAT mount fails with EEXIST where it would succeed on other file systems.
Enable this option to emulate the POSIX behavior: when the destination
exists, it is removed and the rename is retried, with the whole sequence
performed under the same lock as the rename itself. The POSIX rules on what
may replace what are applied first, so that a directory is never silently
removed to make way for a file:
- a directory can only replace an empty directory (ENOTEMPTY otherwise),
- renaming a directory onto a file fails with ENOTDIR,
- renaming a file onto a directory fails with EISDIR.
Without this option all of these fail with EEXIST instead.
Moving a directory into its own subtree is also rejected with EINVAL, as
POSIX requires. f_rename() does not check for this and would link the
directory into its own tree, losing its contents; without this option that
call still succeeds and corrupts the volume.
Note that the emulation is not atomic, and that it cannot honour the POSIX
guarantee that a failed rename leaves an instance of the destination in
place. FAT offers no way to replace a directory entry in a single step, so
an interruption (such as a power loss) between removing the destination and
completing the rename can leave neither name present. Code that must not
lose the destination should copy to a temporary name and rename over it.
config FATFS_USE_DYN_BUFFERS
bool "Use dynamic buffers"
default y

View File

@@ -10,6 +10,7 @@
#include <time.h>
#include <sys/time.h>
#include <sys/unistd.h>
#include <errno.h>
#include <sys/stat.h>
#include "unity.h"
#include "esp_partition.h"
@@ -267,6 +268,171 @@ TEST_CASE("(WL) link copies a file, rename moves a file", "[fatfs][wear_levellin
test_teardown();
}
TEST_CASE("(WL) rename to an existing destination", "[fatfs][wear_levelling]")
{
test_setup();
const char *src = "/spiflash/ren_src.txt";
const char *dst = "/spiflash/ren_dst.txt";
FILE *f = fopen(src, "w");
TEST_ASSERT_NOT_NULL(f);
TEST_ASSERT_TRUE(fputs("source", f) >= 0);
TEST_ASSERT_EQUAL(0, fclose(f));
f = fopen(dst, "w");
TEST_ASSERT_NOT_NULL(f);
TEST_ASSERT_TRUE(fputs("destination", f) >= 0);
TEST_ASSERT_EQUAL(0, fclose(f));
char buf[32];
errno = 0;
int ret = rename(src, dst);
#ifdef CONFIG_FATFS_VFS_RENAME_REPLACES_DESTINATION
/* POSIX behavior: the destination is replaced. */
TEST_ASSERT_EQUAL(0, ret);
memset(buf, 0, sizeof(buf));
f = fopen(dst, "r");
TEST_ASSERT_NOT_NULL(f);
TEST_ASSERT_NOT_NULL(fgets(buf, sizeof(buf), f));
TEST_ASSERT_EQUAL(0, fclose(f));
TEST_ASSERT_EQUAL_STRING("source", buf);
TEST_ASSERT_NULL(fopen(src, "r"));
TEST_ASSERT_EQUAL(0, unlink(dst));
#else
/* Default FatFs behavior: the rename is refused and nothing changes. */
TEST_ASSERT_EQUAL(-1, ret);
TEST_ASSERT_EQUAL(EEXIST, errno);
memset(buf, 0, sizeof(buf));
f = fopen(dst, "r");
TEST_ASSERT_NOT_NULL(f);
TEST_ASSERT_NOT_NULL(fgets(buf, sizeof(buf), f));
TEST_ASSERT_EQUAL(0, fclose(f));
TEST_ASSERT_EQUAL_STRING("destination", buf);
memset(buf, 0, sizeof(buf));
f = fopen(src, "r");
TEST_ASSERT_NOT_NULL(f);
TEST_ASSERT_NOT_NULL(fgets(buf, sizeof(buf), f));
TEST_ASSERT_EQUAL(0, fclose(f));
TEST_ASSERT_EQUAL_STRING("source", buf);
TEST_ASSERT_EQUAL(0, unlink(src));
TEST_ASSERT_EQUAL(0, unlink(dst));
#endif
test_teardown();
}
TEST_CASE("(WL) rename obeys the POSIX rules on directories", "[fatfs][wear_levelling]")
{
test_setup();
const char *file = "/spiflash/ren_f.txt";
const char *dir = "/spiflash/ren_d";
const char *dir2 = "/spiflash/ren_d2";
FILE *f = fopen(file, "w");
TEST_ASSERT_NOT_NULL(f);
TEST_ASSERT_TRUE(fputs("payload", f) >= 0);
TEST_ASSERT_EQUAL(0, fclose(f));
TEST_ASSERT_EQUAL(0, mkdir(dir, 0755));
struct stat st;
/* A file may not replace a directory, and the directory must survive. */
errno = 0;
TEST_ASSERT_EQUAL(-1, rename(file, dir));
#ifdef CONFIG_FATFS_VFS_RENAME_REPLACES_DESTINATION
TEST_ASSERT_EQUAL(EISDIR, errno);
#else
TEST_ASSERT_EQUAL(EEXIST, errno);
#endif
TEST_ASSERT_EQUAL(0, stat(dir, &st));
TEST_ASSERT_TRUE(S_ISDIR(st.st_mode));
/* A directory may not replace a file, and the file must survive. */
errno = 0;
TEST_ASSERT_EQUAL(-1, rename(dir, file));
#ifdef CONFIG_FATFS_VFS_RENAME_REPLACES_DESTINATION
TEST_ASSERT_EQUAL(ENOTDIR, errno);
#else
TEST_ASSERT_EQUAL(EEXIST, errno);
#endif
TEST_ASSERT_EQUAL(0, stat(file, &st));
TEST_ASSERT_FALSE(S_ISDIR(st.st_mode));
/* A directory may not replace a non-empty directory. */
TEST_ASSERT_EQUAL(0, mkdir(dir2, 0755));
f = fopen("/spiflash/ren_d2/occupant.txt", "w");
TEST_ASSERT_NOT_NULL(f);
TEST_ASSERT_EQUAL(0, fclose(f));
errno = 0;
TEST_ASSERT_EQUAL(-1, rename(dir, dir2));
#ifdef CONFIG_FATFS_VFS_RENAME_REPLACES_DESTINATION
TEST_ASSERT_EQUAL(ENOTEMPTY, errno);
#else
TEST_ASSERT_EQUAL(EEXIST, errno);
#endif
TEST_ASSERT_EQUAL(0, stat("/spiflash/ren_d2/occupant.txt", &st));
TEST_ASSERT_EQUAL(0, unlink("/spiflash/ren_d2/occupant.txt"));
/* A directory may replace an empty one. */
errno = 0;
int ret = rename(dir, dir2);
#ifdef CONFIG_FATFS_VFS_RENAME_REPLACES_DESTINATION
TEST_ASSERT_EQUAL(0, ret);
TEST_ASSERT_EQUAL(0, stat(dir2, &st));
TEST_ASSERT_TRUE(S_ISDIR(st.st_mode));
TEST_ASSERT_EQUAL(-1, stat(dir, &st));
TEST_ASSERT_EQUAL(0, rmdir(dir2));
#else
TEST_ASSERT_EQUAL(-1, ret);
TEST_ASSERT_EQUAL(EEXIST, errno);
TEST_ASSERT_EQUAL(0, rmdir(dir));
TEST_ASSERT_EQUAL(0, rmdir(dir2));
#endif
TEST_ASSERT_EQUAL(0, unlink(file));
test_teardown();
}
/* Only meaningful with the option enabled: without it f_rename() happily moves
* the directory into its own tree, which corrupts the volume, so there is no
* safe way to exercise the case. */
#ifdef CONFIG_FATFS_VFS_RENAME_REPLACES_DESTINATION
TEST_CASE("(WL) rename refuses to move a directory into itself", "[fatfs][wear_levelling]")
{
test_setup();
const char *dir = "/spiflash/mv_d";
TEST_ASSERT_EQUAL(0, mkdir(dir, 0755));
struct stat st;
errno = 0;
TEST_ASSERT_EQUAL(-1, rename(dir, "/spiflash/mv_d/child"));
TEST_ASSERT_EQUAL(EINVAL, errno);
TEST_ASSERT_EQUAL(0, stat(dir, &st));
TEST_ASSERT_TRUE(S_ISDIR(st.st_mode));
/* A name that merely shares a prefix is a different directory, and moving
* the directory elsewhere stays allowed. */
TEST_ASSERT_EQUAL(0, rename(dir, "/spiflash/mv_dd"));
TEST_ASSERT_EQUAL(0, stat("/spiflash/mv_dd", &st));
TEST_ASSERT_TRUE(S_ISDIR(st.st_mode));
TEST_ASSERT_EQUAL(0, rmdir("/spiflash/mv_dd"));
test_teardown();
}
#endif // CONFIG_FATFS_VFS_RENAME_REPLACES_DESTINATION
TEST_CASE("(WL) can create and remove directories", "[fatfs][wear_levelling]")
{
test_setup();

View File

@@ -15,6 +15,7 @@ from pytest_embedded_idf.utils import idf_parametrize
'fastseek',
'auto_fsync',
'dyn_buffers',
'posix_rename',
],
)
@idf_parametrize('target', ['esp32', 'esp32c3'], indirect=['target'])

View File

@@ -0,0 +1 @@
CONFIG_FATFS_VFS_RENAME_REPLACES_DESTINATION=y

View File

@@ -981,6 +981,139 @@ cleanup:
return ret;
}
#ifdef CONFIG_FATFS_VFS_RENAME_REPLACES_DESTINATION
/*
* True if `path` names something inside the directory `dir`, that is, `dir` is
* a prefix of `path` ending at a component boundary. FAT names are matched
* case-insensitively, and ASCII case is folded here; a prefix that differs
* only in the case of a non-ASCII character is not recognised, which merely
* leaves such a rename to fail the way it does without this check.
*/
static bool fat_path_is_within(const char *dir, const char *path)
{
size_t i;
for (i = 0; dir[i] != '\0'; i++) {
char a = dir[i];
char b = path[i];
if (b == '\0') {
return false;
}
if (a >= 'a' && a <= 'z') {
a -= 'a' - 'A';
}
if (b >= 'a' && b <= 'z') {
b -= 'a' - 'A';
}
if (a != b) {
return false;
}
}
/* `dir` is exhausted: what follows in `path` decides. A trailing separator
* on `dir` has already consumed the boundary. */
if (i > 0 && dir[i - 1] == '/') {
return path[i] != '\0';
}
return path[i] == '/' && path[i + 1] != '\0';
}
/*
* Handle f_rename() refusing an existing destination, applying the POSIX rules
* for what may replace what. Called with the context lock held and with paths
* that already carry the drive prefix. Returns 0 once the rename has been
* carried out, or the errno to report.
*
* f_rename() reports FR_EXIST only when the destination resolves to a
* different directory entry than the source, so renaming an entry to itself,
* including through another spelling of the same name, never gets here and
* succeeds on its own.
*/
static int vfs_fat_replace_destination(const char *src, const char *dst)
{
FILINFO src_info;
FRESULT fr = f_stat(src, &src_info);
if (fr != FR_OK) {
return fresult_to_errno(fr);
}
FILINFO dst_info;
fr = f_stat(dst, &dst_info);
if (fr != FR_OK) {
return fresult_to_errno(fr);
}
bool src_is_dir = (src_info.fattrib & AM_DIR) != 0;
bool dst_is_dir = (dst_info.fattrib & AM_DIR) != 0;
/* POSIX only allows an entry to be replaced by one of the same kind. This
* has to be checked before removing anything: f_unlink() would happily
* delete an empty directory to make way for a file. */
if (src_is_dir && !dst_is_dir) {
return ENOTDIR;
}
if (!src_is_dir && dst_is_dir) {
return EISDIR;
}
if (dst_info.fattrib & AM_RDO) {
return EACCES;
}
fr = f_unlink(dst);
if (fr == FR_DENIED && dst_is_dir) {
/* A writable directory is only denied removal while it still has
* entries. */
return ENOTEMPTY;
}
if (fr != FR_OK) {
return fresult_to_errno(fr);
}
fr = f_rename(src, dst);
return (fr == FR_OK) ? 0 : fresult_to_errno(fr);
}
static int vfs_fat_rename(void* ctx, const char *src, const char *dst)
{
vfs_fat_ctx_t* fat_ctx = (vfs_fat_ctx_t*) ctx;
_lock_acquire(&fat_ctx->lock);
prepend_drive_to_path(fat_ctx, &src, &dst);
int posix_errno = 0;
if (fat_path_is_within(src, dst)) {
/* Moving a directory inside itself would detach its contents and link
* the directory into its own tree; f_rename() does not check for this
* and would corrupt the volume. POSIX asks for EINVAL, unless the
* source is not a directory at all, in which case the destination
* merely uses a file as a directory component. */
FILINFO src_info;
FRESULT stat_res = f_stat(src, &src_info);
posix_errno = (stat_res != FR_OK) ? fresult_to_errno(stat_res)
: (src_info.fattrib & AM_DIR) ? EINVAL
: ENOTDIR;
} else {
FRESULT res = f_rename(src, dst);
if (res == FR_EXIST) {
posix_errno = vfs_fat_replace_destination(src, dst);
} else if (res != FR_OK) {
posix_errno = fresult_to_errno(res);
}
}
_lock_release(&fat_ctx->lock);
if (posix_errno != 0) {
ESP_LOGD(TAG, "%s: errno=%d", __func__, posix_errno);
errno = posix_errno;
return -1;
}
return 0;
}
#else // CONFIG_FATFS_VFS_RENAME_REPLACES_DESTINATION
static int vfs_fat_rename(void* ctx, const char *src, const char *dst)
{
vfs_fat_ctx_t* fat_ctx = (vfs_fat_ctx_t*) ctx;
@@ -998,6 +1131,8 @@ static int vfs_fat_rename(void* ctx, const char *src, const char *dst)
return 0;
}
#endif // CONFIG_FATFS_VFS_RENAME_REPLACES_DESTINATION
static DIR* vfs_fat_opendir(void* ctx, const char* name)
{
vfs_fat_ctx_t* fat_ctx = (vfs_fat_ctx_t*) ctx;