fat32: fix zero-length files and dot-leading filenames - #419
Conversation
Add coverage for copying a FAT32 partition that holds zero-length files, both file by file with CopyFileSystem and byte for byte with CopyPartitionRaw, asserting the target lists and reads back every entry. A second test closes the loop with the external tools: mkfs.vfat and mcopy write the source, and mdir and fsck.vfat inspect the copy, so the filesystem is created and verified by something other than go-diskfs. It skips when those tools are absent. Two cases fail today. A dot-prefixed name such as .boot_repository is stored with an empty 8.3 base name, which the directory listing then drops, so a file-level copy silently loses the file and CompareFS does not notice. An empty file with no cluster allocated to it — the form mkfs.vfat, mtools and the Linux kernel write — cannot be read at all, so copying any externally created FAT32 aborts with "invalid start cluster: 0". A plain zero-length TEST.TXT created by go-diskfs itself round-trips fine, since go-diskfs allocates a cluster even for an empty file. Signed-off-by: eriknordmark <erik@zededa.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A file with no content has no cluster chain, and every FAT implementation other than go-diskfs records a start cluster of 0 for it. Reading such a file failed outright with "invalid start cluster: 0", so copying any filesystem written by mkfs.vfat, mtools or the Linux kernel aborted as soon as it reached a zero-length file. Reads now return EOF before looking up the chain, the cluster chain of an empty file is reported as empty, and writing to such a file adopts the chain allocated for it. go-diskfs also produces that representation itself now: a new file gets no cluster until it has content, and truncating a file to zero length releases the clusters it held. fsck.vfat reports the previous behavior as "File size is 0 bytes, cluster chain length is > 0 bytes" and exits non-zero. Fixes diskfs#417 Signed-off-by: eriknordmark <erik@zededa.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A file whose name begins with a dot has nothing before the dot to form an 8.3 name from, and was stored with a blank one. fsck.vfat rejects those entries and renames them, and the directory listing skipped them, so such a file was invisible to Stat and to any walk of the filesystem even though it was on disk. It now takes a stem derived from the rest of the name with a numeric tail, matching what mtools and the Linux kernel generate: BOOT_R~1 for .boot_repository, with the existing collision handling picking BOOT_R~2 for the next one. Such names turn up on ordinary boot media: Alpine Linux marks its repository directory with an empty .boot_repository, created by a plain touch in scripts/mkimg.base.sh, so every Alpine image carries one on its FAT32 boot partition. A listing also no longer drops an entry that carries only a long name, so filesystems written before this change stay readable. Signed-off-by: eriknordmark <erik@zededa.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@deitch would you have time to look at this one? I re-checked the two failures against current
The full suite passes on the branch ( One note on the external-tool test: |
|
This looks pretty good. As far as I can tell, you always return empty clusters with zero-length files, but you also ensure that a 0-length file does not allocate a cluster list when created or written. |
The kvm→k conversion tests provisioned their device from the live image only. An installer-written boot disk differs exactly where the conversion is most exposed: its ESP carries a zero-length boot/.boot_repository, and the offline grow relocates the ESP by copying its FAT32 contents, which go-diskfs rejects with "invalid start cluster: 0" before diskfs/go-diskfs#419. A live-image ESP has no such file, so nothing covered that path. Each test gains a FromInstaller sibling differing only in how the boot disk comes into existence. The shared body moves into a helper taking the reuse policy, so the conversion sequence, the readiness gates and every assertion stay common to both. Measured on an installer-provisioned device: the 0-byte marker is present before the resize and intact after it, across an ESP grown from 36 MiB to 2 GiB. Signed-off-by: eriknordmark <erik@zededa.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0156usNe9ABT9Y5ofQH3na8w
Up until now I've been successfully using partitionresizer to shrink and grow partitions used in Alpine Linux, but using live images. When I re-ran that using an installer image created using the mkimg.base in Alpine Linux there were two failures in go-diskfs:
.boot_repositoryin a FAT32 filesystem.Those both come from the zero-length
.boot_repositorycreated bymkimg.base— a plaintouchinscripts/mkimg.base.sh, so the file is both zero length and dot-prefixed.Zero-length files with no allocated cluster
A file with no content has no cluster chain, and every FAT implementation other than go-diskfs records a start cluster of 0 for it.
getClusterListrejected that outright, so reading such a file failed withinvalid start cluster: 0and copying a filesystem aborted at the first zero-length file it reached. Reads now return EOF before looking up the chain, an empty file reports an empty cluster chain, and writing to one adopts the chain allocated for it.go-diskfs also produces that representation itself now, rather than only tolerating it: a new file gets no cluster until it has content, and truncating a file to zero releases the clusters it held.
fsck.vfatreports the previous behavior asFile size is 0 bytes, cluster chain length is > 0 bytesand exits non-zero.Dot-leading filenames
A name beginning with a dot has nothing before the dot to build an 8.3 name from, and was stored with a blank one.
fsck.vfatrejects those entries and renames them (Bad short file name (.BOO)→FSCK0000.000), and the directory listing skipped them, so the file was invisible toStatand to any walk of the filesystem even though it was on disk — a file-level copy dropped it silently. Such a name now takes a stem derived from the rest of the name plus a numeric tail, matching mtools and the Linux kernel:BOOT_R~1for.boot_repository, with the existing collision handling pickingBOOT_R~2for the next one. Listings also no longer drop an entry that has only a long name, so filesystems written by earlier releases stay readable.One deliberate difference from mtools: for a name with a later dot, such as
.config.txt, go-diskfs keeps the stem it already derives and producesCONFIG.TXTwhere mtools producesCONFIG~1.TXT. Both are valid and fsck accepts either; adding a tail unconditionally would change short names for many names that work today. The unit test records the difference.Tests
sync/copy_fat32_test.gocopies a FAT32 partition holding zero-length files into a second partition, both file by file withCopyFileSystemand byte for byte withCopyPartitionRaw, and checks the target lists and reads back every entry. One case patches the source entry's start cluster to zero to get the representation other implementations write, without needing external tools.sync/copy_fat32_mtools_test.gocloses the loop outside go-diskfs:mkfs.vfatandmcopycreate the source filesystem, go-diskfs copies the partition, thenmdirandfsck.vfatinspect the result — so the filesystem under test is written and verified by something other than the code being tested. It skips when those tools are absent. Its partitions are 96 MiB because a 64 MiB FAT32 trips a separate layout bug (#418) that would make the fsck check fail for an unrelated reason.There is also a unit test for the generated 8.3 names and their collision handling, and one asserting that a zero-length file holds no clusters whether it was created empty or truncated back to zero.
Each fix is a separate commit, and the tests come first, so they can be seen failing before either fix and passing after both: four failures at the test commit, two after the cluster fix, none after the short-name fix.