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

Add support for writing long file name to FAT #18

Merged
merged 10 commits into from
Sep 13, 2024

Conversation

xoofx
Copy link

@xoofx xoofx commented Sep 11, 2024

Fixes #15

Opening this PR early on as a draft version with a first version for adding full support for long file names (LFN). It should not be merged yet! πŸ˜…

The tests are passing βœ…, but I will have to add a lot more tests and there are plenty of corner cases that will/might not work yet as expected (e.g changing a short name)

The main change is FileName class converted to a FatFileName struct. This new struct covers the encoding/decoding of LFN+SFN.

Another - smaller - change is in Directory for using more acceleration dictionaries + a free entry acceleration table (in order to handle LFN entries being deleted).

It's gonna take a few more days to polish and make it solid. 🀞

@xoofx
Copy link
Author

xoofx commented Sep 11, 2024

Another aspect are short name collisions, while they should be correctly handled at the FatFileName level, there is more work to do in Directory to support them correctly.

@xoofx
Copy link
Author

xoofx commented Sep 11, 2024

One nice finding discovery that I was able to implement is from http://tomgalvin.uk/blog/gen/2015/06/09/filenames/ where he gives details about how short name hashing for handling collisions is actually implemented in Windows NT+

So the FatFileName is implementing it and it shows similar result to what you could get on a real FAT filesystem on Windows.

@xoofx
Copy link
Author

xoofx commented Sep 11, 2024

Also, the full validation of this PR will require to check that generated FAT disk do work when mounting them on Linux or from a USB stick.

@LTRData
Copy link
Owner

LTRData commented Sep 11, 2024

One nice finding discovery that I was able to implement is from http://tomgalvin.uk/blog/gen/2015/06/09/filenames/ where he gives details about how short name hashing for handling collisions is actually implemented in Windows NT+

So the FatFileName is implementing it and it shows similar result to what you could get on a real FAT filesystem on Windows.

There is really no need for such reverse engineering. Windows NT FAT driver is open source since at least 20 years. The logic for short names is mostly here:
https://github.com/microsoft/Windows-driver-samples/blob/main/filesys/fastfat/dirsup.c

@xoofx
Copy link
Author

xoofx commented Sep 12, 2024

The logic for short names is mostly here:

Not really πŸ™‚ The function used to generate 8.3 name is behind the function RtlGenerate8dot3Name which is not part of this repository but is actually in ntdll.dll (that's what was the goal of the investigation in the post above)

@LTRData
Copy link
Owner

LTRData commented Sep 12, 2024

The logic for short names is mostly here:

Not really πŸ™‚ The function used to generate 8.3 name is behind the function RtlGenerate8dot3Name which is not part of this repository but is actually in ntdll.dll (that's what was the goal of the investigation in the post above)

Ah, I misunderstood what the reverse engineering was about then! Sorry!

@LTRData LTRData changed the base branch from LTRData.DiscUtils-initial to FAT-LFN September 12, 2024 19:54
@xoofx xoofx marked this pull request as ready for review September 12, 2024 20:13
@xoofx
Copy link
Author

xoofx commented Sep 12, 2024

So, I have added several tests, it should have improved coverage on FAT from 70% to 77%. It is far from perfect, the original project is still missing some stuffs here and there, but it's getting better with these tests hopefully. 🀞

I made the PR ready for review to give it more visibility.

Tomorrow early morning I will spend some time to verify that produced FAT filesystem with LFN are working well when mounted from a real disk.

@LTRData
Copy link
Owner

LTRData commented Sep 12, 2024

I just did some experiments with your code. It looks good, I think.

I tried to remove your unsafe keywords and it was pretty easy to get it to work anyway. Just had to add another overload to EncodingExtensions.GetChars that extended Encoding in the same way as the existing one extends Decoder, as well as some other minor tweaks. I think I'll add these changes to a branch I created before merging it.

@xoofx
Copy link
Author

xoofx commented Sep 13, 2024

Tomorrow early morning I will spend some time to verify that produced FAT filesystem with LFN are working well when mounted from a real disk.

I was able to test it on a real disk and seems to work just fine, so this should be good to go!

@LTRData
Copy link
Owner

LTRData commented Sep 13, 2024

Really nice! Thanks a lot for your contribution! πŸ‘

@LTRData LTRData merged commit 2288f31 into LTRData:FAT-LFN Sep 13, 2024
1 check passed
@LTRData
Copy link
Owner

LTRData commented Sep 13, 2024

Looks like tests fail on Linux. Maybe there is a hard coded \ as path separator somewhere. I can check!

@LTRData
Copy link
Owner

LTRData commented Sep 13, 2024

Everything fixed, merged and new NuGet packages are right now uploading.

Again, thanks a lot for your contributions!

@xoofx xoofx deleted the add-support-writing-long-file-name-fat branch September 13, 2024 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Issues with long file names in FAT
2 participants