Add initial support for Fat12 and Fat16 filesystems#294
Add initial support for Fat12 and Fat16 filesystems#294acheronfail wants to merge 2 commits intodiskfs:masterfrom
Conversation
|
I commented on the original issue, but those comments may not hold up in light of your draft here. I still think it is cleaner if we can have If you look at the |
|
I'm happy to have a look at creating separate
Since the However, looking at the current package, I think most of the code actually lives in the I haven't looked too closely, but I suspect that using |
|
As I wrote:
It sounds like you are saying that it is a lot of (probably unnecessary) work to separate it out, so then I would leave it. Just make sure that the |
|
I had some more time and gave it some more work, and I've got most of the tests passing. There's been a significant amount of work required to update the test suites and fixtures to get it all working for I'm beginning to think that splitting them up might be the better option here, since the test code is looking more like spaghetti this way. 😅 I'm not sure how much time I'll have for this over the coming weeks, but I'll try to come back to it when I can. |
|
|
||
| i=0 | ||
| until [ $i -gt 75 ]; do mmd -i /data/fat32.img ::/foo/dir${i}; i=$(( $i+1 )); done | ||
| dd if=/dev/zero of=${out}/disk.img bs=1M count=$(( n + 2 )) |
There was a problem hiding this comment.
This $(( n + 2 )) gives each FAT filesystem enough space to actually be a valid FAT partition.
In the original fixture the FAT32 image was given only 10mb, which resulted in errors such as:
WARNING: Number of clusters for 32 bit FAT is less then suggested minimum.
Warning: Filesystem is FAT32 according to fat_length and fat32_length fields,
but has only 20132 clusters, less than the required minimum of 65525.
This may lead to problems on some systems.
Since it was a "technically incorrect" FAT32 filesystem.
There was a problem hiding this comment.
Yeah, but you always can override it.
This is most definitely a draft right now, but seeking some early feedback.
For the filesystems I'm dealing with, this was able to read a FAT16 volume and list its root directory - which is awesome 🎉 since it's actually not that much code!
I did re-use the
fat32package though, since so much of its code can be re-used.The implementation here is far from complete, things left:
fat32.Filesystem.Type()return different types for fat12 and fat16Create()to handle creating FAT12/16fat32_test.gotests passingMain question before I go any further with this: Should this sit within the
fat32package, or should we split up thefat32package into other packages? Do you have any other suggestions?Fixes #293