:: commit b842720a706b846eeb1110768524829e58502de9

KrekBuk <register@mrgregorix.net> — 2020-05-02 13:47

parents: 6175e7280b

FAT32: support directory entries larger than 1 sector There was already a support for partitions with bigger cluster sizes but there was also a bug where the directory loading code would only look at the first sector of each cluster and could incorrectly report some files as non-existing.

diff --git a/Makefile b/Makefile
index 031a9863..0ec5e4ef 100644
--- a/Makefile
+++ b/Makefile
@@ -62,3 +62,35 @@ fat32-test: all
 	rm -rf test_image loopback_dev
 	./qloader2-install src/qloader2.bin test.img 2048
 	qemu-system-x86_64 -hda test.img -debugcon stdio
+
+fat32-test-big-clusters: all
+	$(MAKE) -C test
+	rm -rf test.img test_image/
+	mkdir test_image
+
+	# Setup partitions
+	dd if=/dev/zero bs=1M count=0 seek=512 of=test.img
+	parted -s test.img mklabel gpt
+	parted -s test.img mkpart primary 2048s 6143s
+	parted -s test.img mkpart primary 6144s 1042432s
+
+	sudo losetup -Pf --show test.img > loopback_dev
+	sudo partprobe `cat loopback_dev`
+	sudo mkfs.fat -s 8 -F 32 `cat loopback_dev`p2
+	sudo mount `cat loopback_dev`p2 test_image
+	sudo mkdir test_image/boot
+
+	# Copy some random files to fill up the fats to span multiple clusters.
+	# This will make the test more credible.
+	sudo find ./src -type f -exec cp -n {} test_image/ \;
+	sudo find ./src -type f -exec cp -n {} test_image/boot/ \;
+
+	# Copy the actual important files
+	sudo cp test/test.elf test_image/boot/
+	sudo cp test/qloader2.cfg test_image/
+	sync
+	sudo umount test_image/
+	sudo losetup -d `cat loopback_dev`
+	rm -rf test_image loopback_dev
+	./qloader2-install src/qloader2.bin test.img 2048
+	qemu-system-x86_64 -hda test.img -debugcon stdio
diff --git a/src/fs/fat32.c b/src/fs/fat32.c
index d2183cb8..208c6002 100644
--- a/src/fs/fat32.c
+++ b/src/fs/fat32.c
@@ -128,57 +128,59 @@ static int fat32_open_in(struct fat32_context* context, struct fat32_directory_e
     bool has_lfn = false;
 
     do {
-        struct fat32_directory_entry directory_entries[FAT32_SECTOR_SIZE / sizeof(struct fat32_directory_entry)];
-        error = fat32_load_fat_cluster_to_memory(context, current_cluster_number, directory_entries, 0, sizeof(directory_entries));
+        for (size_t sector_in_cluster = 0; sector_in_cluster < context->sectors_per_cluster; sector_in_cluster++) {
+            struct fat32_directory_entry directory_entries[FAT32_SECTOR_SIZE / sizeof(struct fat32_directory_entry)];
+            error = fat32_load_fat_cluster_to_memory(context, current_cluster_number, directory_entries, 0 * FAT32_SECTOR_SIZE, sizeof(directory_entries));
 
-        if (error != 0) {
-            return error;
-        }
-
-        for (unsigned int i = 0; i < SIZEOF_ARRAY(directory_entries); i++) {
-            if (directory_entries[i].file_name_and_ext[0] == 0x00) {
-                // no more entries here
-                break;
+            if (error != 0) {
+                return error;
             }
 
-            if (directory_entries[i].attribute == FAT32_LFN_ATTRIBUTE) {
-                has_lfn = true;
+            for (unsigned int i = 0; i < SIZEOF_ARRAY(directory_entries); i++) {
+                if (directory_entries[i].file_name_and_ext[0] == 0x00) {
+                    // no more entries here
+                    break;
+                }
 
-                struct fat32_lfn_entry* lfn = (struct fat32_lfn_entry*) &directory_entries[i];
+                if (directory_entries[i].attribute == FAT32_LFN_ATTRIBUTE) {
+                    has_lfn = true;
 
-                if (lfn->sequence_number & 0b01000000) {
-                    // this lfn is the first entry in the table, clear the lfn buffer
-                    memset(current_lfn, ' ', sizeof(current_lfn));
-                }
+                    struct fat32_lfn_entry* lfn = (struct fat32_lfn_entry*) &directory_entries[i];
+
+                    if (lfn->sequence_number & 0b01000000) {
+                        // this lfn is the first entry in the table, clear the lfn buffer
+                        memset(current_lfn, ' ', sizeof(current_lfn));
+                    }
 
-                const unsigned int lfn_index = ((lfn->sequence_number & 0b00011111) - 1U) * 13U;
-                if (lfn_index >= FAT32_LFN_MAX_ENTRIES * 13) {
+                    const unsigned int lfn_index = ((lfn->sequence_number & 0b00011111) - 1U) * 13U;
+                    if (lfn_index >= FAT32_LFN_MAX_ENTRIES * 13) {
+                        continue;
+                    }
+
+                    fat32_lfncpy(current_lfn + lfn_index + 00, lfn->name1, 5);
+                    fat32_lfncpy(current_lfn + lfn_index + 05, lfn->name2, 6);
+                    fat32_lfncpy(current_lfn + lfn_index + 11, lfn->name3, 2);
                     continue;
                 }
 
-                fat32_lfncpy(current_lfn + lfn_index + 00, lfn->name1, 5);
-                fat32_lfncpy(current_lfn + lfn_index + 05, lfn->name2, 6);
-                fat32_lfncpy(current_lfn + lfn_index + 11, lfn->name3, 2);
-                continue;
-            }
-
-            if (has_lfn) {
-                // remove trailing spaces
-                for (int j = SIZEOF_ARRAY(current_lfn) - 2; j >= -1; j--) {
-                    if (j == -1 || current_lfn[j] != ' ') {
-                        current_lfn[j + 1] = 0;
-                        break;
+                if (has_lfn) {
+                    // remove trailing spaces
+                    for (int j = SIZEOF_ARRAY(current_lfn) - 2; j >= -1; j--) {
+                        if (j == -1 || current_lfn[j] != ' ') {
+                            current_lfn[j + 1] = 0;
+                            break;
+                        }
                     }
                 }
-            }
 
-            if ((has_lfn && strcmp(current_lfn, name) == 0) || strncmp(directory_entries[i].file_name_and_ext, name, 8 + 3) == 0) {
-                *file = directory_entries[i];
-                return 0;
-            }
+                if ((has_lfn && strcmp(current_lfn, name) == 0) || strncmp(directory_entries[i].file_name_and_ext, name, 8 + 3) == 0) {
+                    *file = directory_entries[i];
+                    return 0;
+                }
 
-            if (has_lfn) {
-                has_lfn = false;
+                if (has_lfn) {
+                    has_lfn = false;
+                }
             }
         }
 
tab: 248 wrap: offon