casaroli opened a new pull request, #20799: URL: https://github.com/apache/nuttx/pull/20799
## Summary `rename()` on a mounted file system did not replace an existing target in one step. `mountptrename()` removed the target, then called the rename method of the file system. Between the two calls, another task could find no file at the new path: `open()` failed with `ENOENT`. POSIX requires that the new path always refers to the old file or to the new file. This PR lets the file system replace the target: - `fs/vfs`: `mountptrename()` calls the rename method first. If it fails and the target exists, it removes the target and calls the rename method again, as before. If the removal fails, `rename()` returns the error of the first call. - `fs/tmpfs`: `tmpfs_rename()` replaces an existing file or empty directory in one step (it returned `EEXIST`). An open file that is replaced stays until its last close. A non-empty directory gives `ENOTEMPTY` (before: `EBUSY` from `rmdir`). - `fs/mnemofs`, `fs/userfs`: the rename method did not check for an existing target (mnemofs adds a second entry with the same name; a userfs server can do anything). They now return `EEXIST`, so the VFS keeps the old remove-then-rename path for them. - A short comment in `include/nuttx/fs/fs.h` states what the VFS expects from the rename method. v9fs, hostfs, NFS, littlefs and rpmsgfs already replace the target in one step (host `rename()`, NFS RENAME, `lfs_rename()`). (hostfs `rename()` is broken on master for another reason; a separate PR fixes `hostfs_rename()`.) FAT, SMARTFS and SPIFFS return `EEXIST` and keep the old path. The pseudo file system path holds the inode tree lock for the whole operation and does not change. The first commit fixes white space in `fs_tmpfs.c` that nxstyle reports. These errors are older than this change. ## Impact `rename()` over an existing file is atomic on tmpfs, v9fs, hostfs, NFS, littlefs and rpmsgfs. Other file systems behave as before. No new configuration option. No documentation describes the old behaviour, so only the `fs.h` comment changes. Code size: `lm3s6965-ek:qemu-flat` with `CONFIG_FS_TMPFS`: text +208 bytes. ## Testing `renameatomic.c`: a writer thread renames a new file over the target in a loop for 3 s; a reader thread at a higher priority opens the target every tick. Then it checks the replace rules (file over file, directory over empty and non-empty directory, file over directory, directory over file). `qemu-armv8a:knsh` under HVF, tmpfs mounted by the test, v9fs on `/system`. | | master | this PR | |---|---|---| | tmpfs: `open()` fails with `ENOENT` (4 runs on master, 3 with this PR) | 150/1511, 124/1384, 51/675, 104/1240 | 0/1399, 0/1500, 0/1389 | | v9fs: `open()` fails with `ENOENT` | 414/1470, 287/969, 65/185, 403/1299 | 0/934, 0/932, 0/1117 | | tmpfs: directory over non-empty directory | `EBUSY` | `ENOTEMPTY`, both kept | | other replace rules (tmpfs, v9fs) | pass | pass | ostest passes on `qemu-armv8a:knsh` and `qemu-armv8a:nsh`. `lm3s6965-ek:qemu-flat` builds. `sim:mnemofs` and `sim:userfs` compile the changed files without warnings (the sim link fails on macOS in `sim_hostuart.c`, also on master). `tools/checkpatch.sh -c -u -m -g` passes. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
