Hi Cedric, On 2026-10-26T16:37:26, Cedric Hombourger <[email protected]> wrote:
> fs_write() with an offset trims the file at offset + len. Just to check, this isn't true for all filesystems today. sandbox_fs_write_at() opens the host file without O_TRUNC, so on sandbox hostfs (and semihosting) fs_write() already keeps the tail. FAT and exFAT trim it, and ext4 replaces the whole file. So fs_write() has no defined behaviour for the data after the written range, and its comment in fs.h doesn't mention this. Please can you document in fs.h what fs_write() does with the tail, as part of this series, so callers can see how the two functions differ. > 2. fs: add fs_overwrite(). It uses an optional overwrite() operation in > struct fstype_info. A filesystem without it falls back to write(). I don't like the silent fallback very much. fs_overwrite() promises to keep the tail, but exFAT would still trim the file and ext4 would still replace it, and the caller has no way to tell. I would rather it returned -ENOSYS when the filesystem lacks the operation. EFI can then decide what to do, e.g. fall back explicitly with a comment explaining the limitation. What do you think? > - Naming: fs_overwrite() is what I chose. fs_write_inplace(), > fs_write_keep_tail() and fs_update() were also considered. Please > suggest a better one. To me 'overwrite' sounds like replacing the file, which is closer to what fs_write() does now. How about fs_pwrite(), since the behaviour matches pwrite(2)? (fs_write_keep_tail() is also clear, if a bit long.) > - exFAT has the same trim (exfat_fs_write() calls exfat_truncate() with > offset + size). ext4 refuses a non-zero offset and replaces the file at > offset 0. I did not touch them. Should exFAT get the same operation in > this series? I'd add exFAT in this series. It has the same problem and looks like a small change, since exfat_fs_write() could just skip exfat_truncate() when the write ends before the end of the file. The new operation then isn't FAT-only from the start, and EFI behaves the same on both. ext4 can wait, as long as the fallback question above is settled. > A test for efi_selftest is not included yet. Please can you add one, along with a C test that calls the new fs function on a FAT image. I've made suggestions on patches 2 and 3. That way a later change to set_contents() can't quietly bring the bug back. Regards, Simon
