casaroli opened a new pull request, #21599: URL: https://github.com/apache/nuttx/pull/21599
## Summary Five fixes in the FAT file system, one commit each: - fs/fat: Write at the end of the file for each write with O_APPEND. `fat_open()` moved the file position to the end of the file, but `fat_write()` wrote at the current position. After an `lseek()`, a write with O_APPEND wrote in the middle of the file, and two descriptors with O_APPEND overwrote the data of each other. `fat_write()` now sets the position to the end of the file under the mount lock. It reads O_APPEND from `f_oflags`, as tmpfs does, so `fcntl(F_SETFL)` changes it too (`ff_oflags` is 8 bits wide and cannot hold O_APPEND). - fs/fat: Return the count of a partial write. When `fat_write()` wrote some of the data and then failed, it returned the error. A write that filled the volume gave -1 with ENOSPC, but the file size and the file position included the data, so a retry wrote the same data again. Now it returns the bytes written, and the next write returns ENOSPC, as POSIX requires. - fs/fat: Count the last two clusters in the free cluster count. The data clusters have the FAT entries 2 to `fs_nclusters + 1`. For FAT16 and FAT32, `fat_computefreeclusters()` read `fs_nclusters` entries from entry 0, so `statfs()` gave a free block count that was 2 too low (4331 free clusters on a new FAT16 volume where a file gets 4333). The FAT12 path already reads the right entries. - fs/fat: Report the size of an open file in stat() and fstat(). Both took `st_size` from the directory entry, which gets the size only when the file is synced or closed. So `stat()` and `fstat()` of an open file that was written gave the old size, for example 0 for a new file. If the file is open, the size now comes from its shared state (`fat_shared_find()`, as `fat_open()` does). - fs/fat: Write the zeros of ftruncate() to the volume. When `ftruncate()` makes a file longer, `fat_dirextend()` wrote the zeros of the last sector only into the sector cache of that handle. Each handle has its own sector cache, so another handle of the same file read the old data of the sector from the volume: bytes that an earlier `ftruncate()` cut off, or the data of a deleted file on a new cluster. `fat_truncate()` already writes back and drops the caches of all handles before the extension; `fat_dirextend()` now also writes its last sector back. ## Impact FAT only. The code is the same in flat, protected and kernel builds and on every architecture. What a program sees: - `write()` on a descriptor with O_APPEND always appends, also after `lseek()`, after `fcntl(F_SETFL, O_APPEND)` and from two descriptors. `pwrite()` on a descriptor with O_APPEND now appends, as `fs/vfs/fs_pwrite.c` documents and as tmpfs does. Before, it wrote at the offset. - A write that an error cuts short (for example a full volume) now returns the count of bytes written, not -1. The next write returns the error. - `statfs()` `f_bfree` and `f_bavail` on FAT16 and FAT32 are 2 higher when the last two clusters are free (now the correct value). On FAT32 this is the count computed when FSINFO has no valid count, or at each mount with `CONFIG_FAT_COMPUTE_FSINFO`; the computed count also goes to the FSINFO sector. - `stat()` and `fstat()` of an open file that was written give its current size and the blocks for that size, before a sync. Closed files and directories are as before. - After `ftruncate()` makes a file longer, other descriptors of the file read zeros in the new part. `ftruncate()` to a larger size writes one more sector. - Two descriptors of one file still have separate sector caches, so data that one descriptor writes with `write()` can stay invisible to another descriptor of the file. That is a separate problem, and this PR does not change it. - Size (text): +96 bytes on stm32f4discovery:mmcsdspi (Cortex-M4, flat), +112 bytes on mps2-an500:knsh with FS_FAT (protected). - Documentation/ does not describe these cases for FAT, so there is no documentation change. ## Testing `testing/fs/fatgap` with the new cases of apache/nuttx-apps#NNNNN (it makes FAT12, FAT16 and FAT32 volumes on RAM disks filled with 0xa5): each fix has its own case, and each case fails on master and passes with this PR. | Configuration | master 02ba3229954 (pass / fail) | this PR on c8c0661c997 (pass / fail) | ostest | |---|---|---|---| | qemu-armv8a:nsh (flat), HVF, FAT12/16/32 | 128 / 23 | 151 / 0 | status 0 with both | | qemu-armv8a:knsh (kernel), HVF, FAT12/16 (no memory for the FAT32 RAM disk) | 85 / 15 | 100 / 0 | status 0 with both | The cases that fail on master, on each FAT type: - O_APPEND (commit 1): `append_seek` `offset after the write 2, want 12`; `append_setfl`, `append_pwrite` and `append_twofd` lose the appended data (`size 1024, want 1536` for the two handles). - Partial write (commit 2): `full_volume` `offset 505856, the counts add up to 504000` on FAT12; the last write returned -1 with ENOSPC after it wrote 1856 bytes. - Free count (commit 3): `free_count` `4331 free clusters, a file gets 4333` on FAT16, `68887 ... 68889` on FAT32. FAT12 passes on master. - stat() size (commit 4): `stat_open` `new file: stat: size 0, want 1000`, the same for `fstat()` and a second handle, and `after rename: stat: size 1500, want 1600`. - ftruncate() zeros (commit 5): `trunc_twofd` `second handle: zeros in the sector: byte 100 is 0x61, want 0x00` and `zeros on a new cluster: byte 512 is 0xa1, want 0x00` (FAT16). The write past EOF cases pass with both. The builds have no warnings. - Built, not run: stm32f4discovery:mmcsdspi (flat) and mps2-an500:knsh with FS_FAT (protected). - `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]
