SYS: FAT32 detection should depend on word FAT sector size == zero, only

[ecm-pushbx]

While creating the PR #92 I noticed that SYS determines the FAT type by calculating the number of data clusters then dispatching on that. This is in

kernel/sys/sys.c

Line 1515 in bb1bbbb

else if (clusters < FAT_MAGIC16) /* < 65525 */

However, the kernel itself correctly uses just the zero/nonzero status of the BPB word giving the amount of sectors in a FAT:

kernel/hdr/fat.h

Lines 90 to 93 in bb1bbbb

	/*
	#define _ISFAT32(dpbp) (((dpbp)->dpb_size)>FAT_MAGIC16 && ((dpbp)->dpb_size)<=FAT_MAGIC32 )
	*/
	#define _ISFAT32(dpbp) (((dpbp)->dpb_fatsize)==0)

The kernel is correct: FAT32 is determined solely by whether the BPB word is zero. This also means when calculating the cluster amount (only for dispatching FAT12 vs FAT16) it is correct to use the BPB word, not the EBPB dword, to calculate the space occupied by the FATs. SYS already does that correctly by chance, though it could be clearer about it. In

kernel/sys/sys.c

Line 1507 in bb1bbbb

fatSize = bs32->bsFATsecs ? bs32->bsFATsecs : bs32->bsBigFatSize;

[ecm-pushbx]

I discussed this in dosemu2/fdpp#202 (comment) too.

[ecm-pushbx]

Quoting me from that thread:

mtools just had this edge case fixed. The truth is that the word (16-bit) "sectors per FAT" field being zero indicates using both the larger EBPB and FAT32. It doesn't really matter how many clusters there are, you can create a FAT32 file system with less than 64 Ki clusters. That seems to be what most drivers, including Microsoft's, actually do.

Quoting from the mailing list for mtools: https://lists.gnu.org/archive/html/info-mtools/2022-09/msg00003.html

Actually, according to Microsoft's specification, number of FAT bits of
an existing filesystem is SOLELY determined by the number of clusters.
This applies to all three bit numbers: 12/16/32

Yes, the (in)famous fatgen103.pdf document, retired from Microsoft's own
site, but still available at
https://web.archive.org/web/20210723100623/http://download.microsoft.com/download/1/6/1/161ba512-40e2-4cc9-843a-923143f3456c/fatgen103.doc
It sternly says at page 14:
"It is really quite simple how this works. The FAT type— one of FAT12,
FAT16, or FAT32—is determined by the count of clusters on the volume
and nothing else."
But see below...:
[...]

But as Alain pointed Microsoft FAT implementation (fastfat.sys) for
FAT32 detection does not use number of clusters, but rather explicit
marking of FAT32. Same behavior has Linux kernel implementation
(msdos.ko and vfat.ko) and same in dosfstools project. Hence this
non-standard FAT32 can be still created by mkfs.fat.

Actually, what I meant in my previous mail was that everything else
that is referred to in the FAT32 extended boot block (root directory
location, etc.) would depend on this explicit marking
(fatLen/bigFatLen), but not the number of FAT bits themselves.
However, your remark got me thinking, and so I tried it out on a Windows
10 VM.
... and lo and behold: the mere presence of a FAT32 extended boot block
(as signaled by fat_len being 0) was enough to make it use 32 fat bits,
even if the number of clusters was too low for FAT32.
Sorry for having been to easily misled by an assertive, but nonfactual,
Microsoft statement.
Given this finding, I'll shortly make a new mtools release that will
consider presence of FAT32 extended boot block as well for picking
appropriate number of FAT bits.

if (DiskTransferBuffer[0x26] == 0x29)         // BPB v4.1
    offset = 0x27;

This seems wrong. If you have a FAT32 EBPB then the byte at offset 26h is part of the ebpbSectorsPerFATLarge field, for which 29h would be a valid value. You should check bpbSectorsPerFAT == 0 to determine whether to use the EBPB or BPB new fields.