Bugfix filesystem reads at cluster boundary

include/fs/fat32.h

@@ -332,6 +332,10 @@ struct fat32_handle {
      * @brief the first cluster of this directory or file for convenience purposes
      */
     uint32_t first_cluster;
+    /**
+     * @brief the how manieth cluster in the chain is currently stored in the current_cluster field (0-indexed)
+     */
+    size_t current_cluster_index_in_chain;
     /**
      * @brief cluster containing the current byte offset
      *

lib/fs/fat32.c

@@ -856,6 +856,7 @@ static errval_t fat32_create_handle(struct fat32 *fat32, struct fat32_handle **h
     (*handle)->is_dir = is_dir;
     (*handle)->first_cluster = first_cluster;
     (*handle)->current_cluster = first_cluster;
+    (*handle)->current_cluster_index_in_chain = 0;
     (*handle)->byte_offset = 0;
     (*handle)->directory_entry_ref = directory_entry_ref;
 
@@ -879,6 +880,44 @@ static errval_t fat32_free_handle(struct fat32 *fat32, struct fat32_handle *hand
     return SYS_ERR_OK;
 }
 
+static errval_t fat32_handle_update_current_cluster(struct fat32 *fat32, struct fat32_handle *handle, bool extend) {
+    errval_t err;
+
+    // debug_printf("[fat32_handle_update_current_cluster]\n");
+    
+    size_t cluster_index = handle->byte_offset / fat32->bytes_per_cluster;
+
+    // debug_printf("[fat32_handle_update_current_cluster] target cluster %lu for offset %lu\n", cluster_index, handle->byte_offset);
+
+    if (handle->current_cluster_index_in_chain == cluster_index) return SYS_ERR_OK;
+
+    // if we are too far in the chain then we have to re-resolve from the start
+    if (handle->current_cluster_index_in_chain > cluster_index) {
+        handle->current_cluster_index_in_chain = 0;
+        handle->current_cluster = handle->first_cluster;
+    }
+
+    // advance in the cluster chain until we reach the desired index or run out of clusters
+    assert(!fat32_is_end_of_cluster_chain(handle->current_cluster));
+    while(handle->current_cluster_index_in_chain < cluster_index) {
+        if (extend) {
+            err = fat32_next_cluster_in_chain_or_extend(fat32, handle->current_cluster, &handle->current_cluster);
+            if (err_is_fail(err)) return err;
+            assert(!fat32_is_end_of_cluster_chain(handle->current_cluster));
+        } else {
+            uint32_t next_cluster;
+            err = fat32_next_cluster_in_chain(fat32, handle->current_cluster, &next_cluster);
+            if (err_is_fail(err)) return err;
+
+            if(fat32_is_end_of_cluster_chain(next_cluster)) return FS_ERR_INDEX_BOUNDS;
+            handle->current_cluster = next_cluster;
+        }
+        ++handle->current_cluster_index_in_chain;
+    }
+
+    return SYS_ERR_OK;
+}
+
 /**
  * @brief Verify contents of the bootsector of the SD Card
  * 
@@ -1046,7 +1085,7 @@ errval_t fat32_init(
     fat32->max_cluster_number = ((bpb->total_sector_32_count - fat32->first_data_sector) / bpb->sectors_per_cluster) + 1;
     fat32->bytes_per_cluster = fat32->bytes_per_sector * fat32->sectors_per_cluster;
 
-    fat32_print(bpb);
+    // fat32_print(bpb);
     fat32_verify(bpb);
 
     debug_printf("[fat32_init] verified FAT32\n");
@@ -1274,6 +1313,10 @@ static errval_t _fat32_readdir(struct fat32 *fat32, struct fat32_handle *dir_han
         return FS_ERR_INVALID_FH;
     }
 
+    // follow the chain if the current cluster is not defined
+    err = fat32_handle_update_current_cluster(fat32, dir_handle, false);
+    if(err_is_fail(err)) return err;
+
     *name = NULL;
 
     // search for the next file in the directory starting at the current byte offset
@@ -1302,8 +1345,8 @@ static errval_t _fat32_readdir(struct fat32 *fat32, struct fat32_handle *dir_han
                 return SYS_ERR_OK;
             }
         }
-        err = fat32_next_cluster_in_chain(fat32, dir_handle->current_cluster, &dir_handle->current_cluster);
+        err = fat32_handle_update_current_cluster(fat32, dir_handle, false);
-        if (err_is_fail(err)) return err;
+        if(err_is_fail(err)) return err;
     }
 
     return FS_ERR_INDEX_BOUNDS;
@@ -1535,17 +1578,18 @@ errval_t fat32_fopen(struct fat32 *fat32, const char *path, struct fat32_handle
 }
 
 static errval_t _fat32_fread(struct fat32 *fat32, struct fat32_handle *file_handle, void *buffer, size_t bytes, size_t *bytes_read) {
-    // debug_printf("[fat32_fread] on %p: %lu bytes from offset %lu\n", file_handle, bytes, file_handle->byte_offset);
+    debug_printf("[fat32_fread] on %p: %lu bytes from offset %lu\n", file_handle, bytes, file_handle->byte_offset);
     errval_t err;
 
     if (file_handle == NULL || file_handle->is_dir) return FS_ERR_INVALID_FH;
 
     struct fat32_directory_entry directory_entry;
+    // debug_printf("[fat32_fread] read directory entry\n");
     err = fat32_read_directory_entry(fat32, file_handle->directory_entry_ref, &directory_entry);
     if (err_is_fail(err)) return err;
 
     // trying to read outside of the file
-    if (file_handle->byte_offset >= directory_entry.file_size) {
+    if (bytes == 0 || file_handle->byte_offset >= directory_entry.file_size) {
         *bytes_read = 0;
         return SYS_ERR_OK;
     }
@@ -1558,23 +1602,14 @@ static errval_t _fat32_fread(struct fat32 *fat32, struct fat32_handle *file_hand
         // debug_printf("[fat32_fread] attempted to read over end of file, reading only %lu/%lu\n", bytes_to_read, bytes);
     // }
 
-    // follow the chain if the current cluster is not defined
+    // make sure we have the correct current cluster
-    if (file_handle->current_cluster == 0) {
+    err = fat32_handle_update_current_cluster(fat32, file_handle, false);
-        size_t cluster_min_count = ROUND_UP(file_handle->byte_offset, fat32->bytes_per_cluster) / fat32->bytes_per_cluster;
+    if (err_is_fail(err)) return err;
-
-        // update the current_cluster in the file handle without extending the chain
-        file_handle->current_cluster = file_handle->first_cluster;
-        for (size_t i = 0; i + 1 < cluster_min_count; ++i) {
-            err = fat32_next_cluster_in_chain(fat32, file_handle->current_cluster, &file_handle->current_cluster);
-            if (err_is_fail(err)) return err;
-        }
-        assert(!fat32_is_end_of_cluster_chain(file_handle->current_cluster));
-    }
 
     // read until we have reached the desired amount of bytes
     // where we read is determined by the current byte offset of the file handle
     *bytes_read = 0;
-    while (*bytes_read < bytes_to_read) {
+    while (true) {
         // make sure we never try to look at the reserved clusters during traversal
         assert(file_handle->current_cluster > 1);
         assert(!fat32_is_end_of_cluster_chain(file_handle->current_cluster));
@@ -1591,6 +1626,7 @@ static errval_t _fat32_fread(struct fat32 *fat32, struct fat32_handle *file_hand
             size_t offset_in_sector = file_handle->byte_offset % fat32->bytes_per_sector;
             size_t bytes_left_in_sector = FAT32_BLOCK_SIZE - offset_in_sector;
             size_t bytes_in_sector = MIN(bytes_left_in_sector, bytes_to_read - *bytes_read);
+            // debug_printf("[fat32_fread] reading %lu from sector %u due to offset %u\n", bytes_in_sector, current_sector, sector_offset);
             err = fat32->read_object_fn(current_sector, offset_in_sector, bytes_in_sector, (buffer + *bytes_read));
             if (err_is_fail(err)) return err;
 
@@ -1603,15 +1639,13 @@ static errval_t _fat32_fread(struct fat32 *fat32, struct fat32_handle *file_hand
             }
         }
         // continue in the next cluster
-        err = fat32_next_cluster_in_chain(fat32, file_handle->current_cluster, &file_handle->current_cluster);
+        err = fat32_handle_update_current_cluster(fat32, file_handle, false);
-        if (err_is_fail(err)) return err;
+        if(err_is_fail(err)) return err;
         // we have checked in advance that this exists by checking the file size
         assert(!fat32_is_end_of_cluster_chain(file_handle->current_cluster));
     }
 
-    // debug_printf("[fat32_fread] file now at offset %lu\n", file_handle->byte_offset);
+    USER_PANIC("Unreachable code");
-
-    return SYS_ERR_OK;
 }
 errval_t fat32_fread(struct fat32 *fat32, struct fat32_handle *file_handle, void *buffer, size_t bytes, size_t *bytes_read) {
     errval_t err;
@@ -1640,21 +1674,8 @@ static errval_t _fat32_fwrite(struct fat32 *fat32, struct fat32_handle *file_han
         return FS_ERR_WRITE;
     }
 
-    // if the current position in the file is past the end we need to increase the file size accordingly
+    // make sure we have a valid current cluster
-    if (file_handle->byte_offset >= directory_entry.file_size || file_handle->current_cluster == 0) {
+    err = fat32_handle_update_current_cluster(fat32, file_handle, true);
-        // debug_printf("Invalid current cluster %u, updating it: %u (%lu)\n", file_handle->current_cluster, file_handle->first_cluster, file_handle->byte_offset);
-        // find the cluster containing the current position in the file
-        size_t cluster_min_count = ROUND_UP(file_handle->byte_offset, fat32->bytes_per_cluster) / fat32->bytes_per_cluster;
-
-        // when we are past the end of the file then the cluster_number is invalid
-        file_handle->current_cluster = file_handle->first_cluster;
-        for (size_t i = 0; i + 1 < cluster_min_count; ++i) {
-            // add a cluster to the chain if necessary
-            err = fat32_next_cluster_in_chain_or_extend(fat32, file_handle->current_cluster, &file_handle->current_cluster);
-            if (err_is_fail(err)) return err;
-        }
-        // debug_printf("current_cluster: %u\n", file_handle->current_cluster);
-    };
 
     // write the data to the file
     *bytes_written = 0;
@@ -1688,10 +1709,11 @@ static errval_t _fat32_fwrite(struct fat32 *fat32, struct fat32_handle *file_han
         if (err_is_fail(err)) break;
         if (*bytes_written == bytes) break;
 
-        // if we reached the end of the file just extend it
+        // continue in the next cluster
-        err = fat32_next_cluster_in_chain_or_extend(fat32, file_handle->current_cluster, &file_handle->current_cluster);
+        err = fat32_handle_update_current_cluster(fat32, file_handle, true);
-        if (err_is_fail(err)) break;
+        if(err_is_fail(err)) return err;
-    }
+        // we extend the chain if we reach the end so we should never teach the end of the chain
+        assert(!fat32_is_end_of_cluster_chain(file_handle->current_cluster));    }
 
     // if we had to increase the file size write it in the directory
     if (directory_entry.file_size < file_handle->byte_offset) {
@@ -1791,20 +1813,6 @@ static errval_t _fat32_seek(struct fat32 *fat32, struct fat32_handle *file_handl
 
     file_handle->byte_offset = absolute_offset;
 
-    // find the cluster containing the current position in the file
-    size_t cluster_min_count = ROUND_UP(file_handle->byte_offset, fat32->bytes_per_cluster) / fat32->bytes_per_cluster;
-
-    // update the current_cluster in the file handle without extending the chain
-    file_handle->current_cluster = file_handle->first_cluster;
-    for (size_t i = 0; i + 1 < cluster_min_count && !fat32_is_end_of_cluster_chain(file_handle->current_cluster); ++i) {
-        err = fat32_next_cluster_in_chain(fat32, file_handle->current_cluster, &file_handle->current_cluster);
-        if (err_is_fail(err)) return err;
-    }
-    // we are over the end of the cluster chain, so invalidate the current_cluster
-    if (fat32_is_end_of_cluster_chain(file_handle->current_cluster)) {
-        file_handle->current_cluster = 0;
-    }
-
     return SYS_ERR_OK;
 }
 errval_t fat32_seek(struct fat32 *fat32, struct fat32_handle *file_handle, enum fs_seekpos whence, off_t offset) {