Nextor icon indicating copy to clipboard operation
Nextor copied to clipboard

FAT size rounding issue in fdisk?

Open grauw opened this issue 2 years ago • 5 comments

On the if here:

https://github.com/Konamiman/Nextor/blob/v2.1/source/kernel/bank5/fdisk2.c#L416

sectorsPerFat = (clusterCount + 2) >> 8;

if(((clusterCount + 2) & 0x3FF) != 0) {
	sectorsPerFat++;
}

If the intent is to round up the division above it (>> 8), should this not be 0xFF?

It looks like a copy / pasta from the FAT12 code, but the calculation there is different.

grauw avatar Jan 07 '23 01:01 grauw

Additionally, should PARTYPE_EXTENDED not be 0x0F instead of 0x05? As I understand it that’s the type to use for LBA addressing, which seems appropriate since FDISK writes only LBA information. If that’s changed the kernel should be as well.

Lastly, if I understand the wiki correctly, it also says that the second (link) EBR entry’s size field is always the size of the next partition, whereas FDISK sets the first one to the total size of the entire EBR chain. I wonder which is correct.

grauw avatar Jan 16 '23 00:01 grauw

Hi @grauw, thanks for your report!

Additionally, should PARTYPE_EXTENDED not be 0x0F instead of 0x05

Probably, but I don't think it would be a good idea to change that at this point (for new partitions created with FDISK) as that could cause issues with older versions of MAPDRV (and apparently, using 0x05 instead of 0x0F hasn't caused any issues so far).

That said, what Nextor should definitely do is recognizing 0x0F as an equivalent to 0x05 when searching for partitions in devices; and that's exactly what https://github.com/Konamiman/Nextor/pull/121 does (feel free to review if you want).

Lastly, if I understand the wiki correctly, it also says that the second (link) EBR entry’s size field is always the size of the next partition, whereas FDISK sets the first one to the total size of the entire EBR chain. I wonder which is correct.

I wonder too as in the past I have found conflicting information regarding this. Anyway that specific field of the partition table isn't effectively used by Nextor, and apparently neither by other OSes; so I'll leave that part as is unless a specific issue is detected.

Konamiman avatar Apr 16 '23 19:04 Konamiman

Hello, @Konamiman

Additionally, should PARTYPE_EXTENDED not be 0x0F instead of 0x05

Probably, but I don't think it would be a good idea to change that at this point (for new partitions created with FDISK) as that could cause issues with older versions of MAPDRV (and apparently, using 0x05 instead of 0x0F hasn't caused any issues so far).

That said, what Nextor should definitely do is recognizing 0x0F as an equivalent to 0x05 when searching for partitions in devices; and that's exactly what #121 does (feel free to review if you want).

Well, yes, the troubles pop up only when media, partitioned with NEXTOR's fdisk, are accessed outside MSX, on systems that practically respect the difference between EXTENDED and EXTENDED LBA.

ATroubleshooter avatar Apr 19 '23 10:04 ATroubleshooter

Well, yes, the troubles pop up only when media, partitioned with NEXTOR's fdisk, are accessed outside MSX, on systems that practically respect the difference between EXTENDED and EXTENDED LBA.

On Linux, for instance, you get this: partx -a parttest.dsk partx: /dev/loop17: error adding partition 5

ATroubleshooter avatar Apr 19 '23 11:04 ATroubleshooter

the troubles pop up only when media, partitioned with NEXTOR's fdisk, are accessed outside MSX, on systems that practically respect the difference between EXTENDED and EXTENDED LBA.

That makes sense. I think I'll change it so that FDISK creates partitions with the LBA code, and will also create a tool to change the code in existing partitions to either the old or the new value.

Konamiman avatar Oct 29 '23 11:10 Konamiman