Hi Cedric,
On 2026-10-26T16:37:26, Cedric Hombourger <[email protected]> wrote:
> fs: fat: add a write that keeps the end of the file
>
> A write at an offset that ends before the end of the file trims the
> file at that point. This was chosen on purpose in commit cb8af8af5ba0
> ("fs: fat: support write with non-zero offset") to stay compatible with
> the interface of the fatwrite command, which replaces the whole file.
>
> Callers that update a part of a file, like the EFI file protocol, need
> the data after the written range to stay.
>
> Move the body of file_fat_write_at() to a static function that takes a
> keep_tail flag. With the flag set, set_contents() does not free the
> clusters after the written range, and the size in the directory entry
> only grows. Keep file_fat_write_at() as a wrapper with the current
> behaviour and add file_fat_overwrite() for the new one.
>
> Signed-off-by: Cedric Hombourger <[email protected]>
>
> fs/fat/fat_write.c | 32 ++++++++++++++++++++++++--------
> include/fat.h | 2 ++
> 2 files changed, 26 insertions(+), 8 deletions(-)
> diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
> @@ -1535,7 +1537,8 @@ int file_fat_write_at(const char *filename, loff_t pos,
> void *buffer,
> /* Update file size in a directory entry */
> - retdent->size = cpu_to_le32(pos + size);
> + if (!keep_tail || pos + size > FAT2CPU32(retdent->size))
> + retdent->size = cpu_to_le32(pos + size);
One thing I'd like to check is the zero-length case. With pos == 0 and
size == 0, set_contents() takes the '!filesize' path at the top, which
frees the whole cluster chain and sets the start cluster to 0 without
looking at keep_tail. The directory entry keeps its old size, so you
end up with a non-zero size and no clusters. EFI is safe because
efi_file_write_int() returns early when *buffer_size is 0, but
file_fat_overwrite() is now a public entry point. Please can you make
a zero-length write a no-op when keep_tail is set, either here or in
set_contents()?
> diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
> @@ -1578,6 +1582,18 @@ exit:
> +int file_fat_overwrite(const char *filename, void *buffer, loff_t offset,
> + loff_t maxsize, loff_t *actwrite)
> +{
> + return fat_write_at(filename, offset, buffer, maxsize, actwrite, true);
> +}
On naming, see my reply to the cover letter. Here,
file_fat_write_inplace() would describe the intent better.
Regards,
Simon
