Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FAT size rounding issue in fdisk? #113

Open
grauw opened this issue Jan 7, 2023 · 5 comments · Fixed by #121
Open

FAT size rounding issue in fdisk? #113

grauw opened this issue Jan 7, 2023 · 5 comments · Fixed by #121
Labels
due in next version This will be fixed/implemented in the next version

Comments

@grauw
Copy link

grauw commented Jan 7, 2023

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 grauw changed the title FAT size rounding issue? FAT size rounding issue in fdisk? Jan 7, 2023
@grauw
Copy link
Author

grauw commented Jan 16, 2023

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.

@Konamiman
Copy link
Owner

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 #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.

@ATroubleshooter
Copy link

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
Copy link

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

@Konamiman Konamiman added the due in next version This will be fixed/implemented in the next version label Oct 26, 2023
@Konamiman
Copy link
Owner

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
due in next version This will be fixed/implemented in the next version
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants