:: commit 130942434646d64bdd6a87f9181823120bb516a1

mintsuki <mintsuki@protonmail.com> — 2022-07-04 18:16

parents: 387b11e4dd

fs: Improve layering in how the filesystem API works

diff --git a/common/fs/echfs.h b/common/fs/echfs.h
index bafce9b2..50d08cdb 100644
--- a/common/fs/echfs.h
+++ b/common/fs/echfs.h
@@ -1,43 +1,12 @@
 #ifndef __FS__ECHFS_H__
 #define __FS__ECHFS_H__
 
-#include <stdint.h>
 #include <stdbool.h>
 #include <lib/part.h>
-#include <lib/blib.h>
+#include <fs/file.h>
 
-struct echfs_dir_entry {
-    uint64_t parent_id;
-    uint8_t type;
-    char name[201];
-    uint64_t atime;
-    uint64_t mtime;
-    uint16_t perms;
-    uint16_t owner;
-    uint16_t group;
-    uint64_t ctime;
-    uint64_t payload;
-    uint64_t size;
-} __attribute__((packed));
-
-struct echfs_file_handle {
-    struct volume *part;
-    uint64_t block_size;
-    uint64_t block_count;
-    uint64_t dir_length;
-    uint64_t alloc_table_size;
-    uint64_t alloc_table_offset;
-    uint64_t dir_offset;
-    uint64_t file_block_count;
-    uint64_t *alloc_map;
-    struct echfs_dir_entry dir_entry;
-};
-
-int echfs_check_signature(struct volume *part);
 bool echfs_get_guid(struct guid *guid, struct volume *part);
 
-bool echfs_open(struct echfs_file_handle *ret, struct volume *part, const char *filename);
-void echfs_read(struct echfs_file_handle *file, void *buf, uint64_t loc, uint64_t count);
-void echfs_close(struct echfs_file_handle *file);
+struct file_handle *echfs_open(struct volume *part, const char *filename);
 
 #endif
diff --git a/common/fs/echfs.s2.c b/common/fs/echfs.s2.c
index c14546b6..5c68682a 100644
--- a/common/fs/echfs.s2.c
+++ b/common/fs/echfs.s2.c
@@ -7,6 +7,33 @@
 #include <stdbool.h>
 #include <mm/pmm.h>
 
+struct echfs_dir_entry {
+    uint64_t parent_id;
+    uint8_t type;
+    char name[201];
+    uint64_t atime;
+    uint64_t mtime;
+    uint16_t perms;
+    uint16_t owner;
+    uint16_t group;
+    uint64_t ctime;
+    uint64_t payload;
+    uint64_t size;
+} __attribute__((packed));
+
+struct echfs_file_handle {
+    struct volume *part;
+    uint64_t block_size;
+    uint64_t block_count;
+    uint64_t dir_length;
+    uint64_t alloc_table_size;
+    uint64_t alloc_table_offset;
+    uint64_t dir_offset;
+    uint64_t file_block_count;
+    uint64_t *alloc_map;
+    struct echfs_dir_entry dir_entry;
+};
+
 struct echfs_identity_table {
     uint8_t  jmp[4];
     char     signature[8];
@@ -26,7 +53,9 @@ static bool read_block(struct echfs_file_handle *file, void *buf, uint64_t block
     return volume_read(file->part, buf, (file->alloc_map[block] * file->block_size) + offset, count);
 }
 
-void echfs_read(struct echfs_file_handle *file, void *buf, uint64_t loc, uint64_t count) {
+static void echfs_read(struct file_handle *h, void *buf, uint64_t loc, uint64_t count) {
+    struct echfs_file_handle *file = h->fd;
+
     for (uint64_t progress = 0; progress < count;) {
         uint64_t block = (loc + progress) / file->block_size;
 
@@ -40,17 +69,6 @@ void echfs_read(struct echfs_file_handle *file, void *buf, uint64_t loc, uint64_
     }
 }
 
-int echfs_check_signature(struct volume *part) {
-    struct echfs_identity_table id_table;
-    volume_read(part, &id_table, 0, sizeof(struct echfs_identity_table));
-
-    if (strncmp(id_table.signature, "_ECH_FS_", 8)) {
-        return 0;
-    }
-
-    return 1;
-}
-
 bool echfs_get_guid(struct guid *guid, struct volume *part) {
     struct echfs_identity_table id_table;
     volume_read(part, &id_table, 0, sizeof(struct echfs_identity_table));
@@ -64,22 +82,25 @@ bool echfs_get_guid(struct guid *guid, struct volume *part) {
     return true;
 }
 
-void echfs_close(struct echfs_file_handle *file) {
-    pmm_free(file->alloc_map, file->file_block_count * sizeof(uint64_t));
-    pmm_free(file, sizeof(struct echfs_file_handle));
+static void echfs_close(struct file_handle *file) {
+    struct echfs_file_handle *f = file->fd;
+    pmm_free(f->alloc_map, f->file_block_count * sizeof(uint64_t));
+    pmm_free(f, sizeof(struct echfs_file_handle));
+    pmm_free(file, sizeof(struct file_handle));
 }
 
-bool echfs_open(struct echfs_file_handle *ret, struct volume *part, const char *path) {
-    ret->part = part;
-
+struct file_handle *echfs_open(struct volume *part, const char *path) {
     struct echfs_identity_table id_table;
-    volume_read(ret->part, &id_table, 0, sizeof(struct echfs_identity_table));
+    volume_read(part, &id_table, 0, sizeof(struct echfs_identity_table));
 
     if (strncmp(id_table.signature, "_ECH_FS_", 8)) {
-        print("echfs: signature invalid\n");
-        return false;
+        return NULL;
     }
 
+    struct echfs_file_handle *ret = ext_mem_alloc(sizeof(struct echfs_file_handle));
+
+    ret->part = part;
+
     ret->block_size         = id_table.block_size;
     ret->block_count        = id_table.block_count;
     ret->dir_length         = id_table.dir_length * ret->block_size;
@@ -124,7 +145,8 @@ next:;
         }
     }
 
-    return false;
+    pmm_free(ret, sizeof(struct echfs_file_handle));
+    return NULL;
 
 found:;
     // Load the allocation map.
@@ -141,5 +163,16 @@ found:;
             sizeof(uint64_t));
     }
 
-    return true;
+    struct file_handle *handle = ext_mem_alloc(sizeof(struct file_handle));
+
+    handle->fd = ret;
+    handle->read = (void *)echfs_read;
+    handle->close = (void *)echfs_close;
+    handle->size = ret->dir_entry.size;
+    handle->vol = part;
+#if uefi == 1
+    handle->efi_part_handle = part->efi_part_handle;
+#endif
+
+    return handle;
 }
diff --git a/common/fs/ext2.h b/common/fs/ext2.h
index ad75a5c3..22f787ac 100644
--- a/common/fs/ext2.h
+++ b/common/fs/ext2.h
@@ -1,133 +1,13 @@
 #ifndef __FS__EXT2_H__
 #define __FS__EXT2_H__
 
-#include <stdint.h>
-#include <stddef.h>
 #include <stdbool.h>
 #include <lib/part.h>
-#include <lib/blib.h>
+#include <fs/file.h>
 
-/* Superblock Fields */
-struct ext2_superblock {
-    uint32_t s_inodes_count;
-    uint32_t s_blocks_count;
-    uint32_t s_r_blocks_count;
-    uint32_t s_free_blocks_count;
-    uint32_t s_free_inodes_count;
-    uint32_t s_first_data_block;
-    uint32_t s_log_block_size;
-    uint32_t s_log_frag_size;
-    uint32_t s_blocks_per_group;
-    uint32_t s_frags_per_group;
-    uint32_t s_inodes_per_group;
-    uint32_t s_mtime;
-    uint32_t s_wtime;
-
-    uint16_t s_mnt_count;
-    uint16_t s_max_mnt_count;
-    uint16_t s_magic;
-    uint16_t s_state;
-    uint16_t s_errors;
-    uint16_t s_minor_rev_level;
-
-    uint32_t s_lastcheck;
-    uint32_t s_checkinterval;
-    uint32_t s_creator_os;
-    uint32_t s_rev_level;
-    uint16_t s_def_resuid;
-    uint16_t s_def_gid;
-
-    // if version number >= 1, we have to use the ext2 extended superblock as well
-
-    /* Extended Superblock */
-    uint32_t s_first_ino;
-
-    uint16_t s_inode_size;
-    uint16_t s_block_group_nr;
-
-    uint32_t s_feature_compat;
-    uint32_t s_feature_incompat;
-    uint32_t s_feature_ro_compat;
-
-    uint64_t s_uuid[2];
-    uint8_t s_volume_name[16];
-
-    uint64_t s_last_mounted[8];
-
-    uint32_t compression_info;
-    uint8_t prealloc_blocks;
-    uint8_t prealloc_dir_blocks;
-    uint16_t reserved_gdt_blocks;
-    uint8_t journal_uuid[16];
-    uint32_t journal_inum;
-    uint32_t journal_dev;
-    uint32_t last_orphan;
-    uint32_t hash_seed[4];
-    uint8_t def_hash_version;
-    uint8_t jnl_backup_type;
-    uint16_t group_desc_size;
-    uint32_t default_mount_opts;
-    uint32_t first_meta_bg;
-    uint32_t mkfs_time;
-    uint32_t jnl_blocks[17];
-} __attribute__((packed));
-
-struct ext2_linux {
-    uint8_t  frag_num;
-    uint8_t  frag_size;
-
-    uint16_t reserved_16;
-    uint16_t user_id_high;
-    uint16_t group_id_high;
-
-    uint32_t reserved_32;
-} __attribute__((packed));
-
-struct ext2_inode {
-    uint16_t i_mode;
-    uint16_t i_uid;
-
-    uint32_t i_size;
-    uint32_t i_atime;
-    uint32_t i_ctime;
-    uint32_t i_mtime;
-    uint32_t i_dtime;
-
-    uint16_t i_gid;
-    uint16_t i_links_count;
-
-    uint32_t i_blocks_count;
-    uint32_t i_flags;
-    uint32_t i_osd1;
-    uint32_t i_blocks[15];
-    uint32_t i_generation;
-
-    /* EXT2 v >= 1.0 */
-    uint32_t i_eab;
-    uint32_t i_maj;
-
-    /* EXT2 vAll */
-    uint32_t i_frag_block;
-
-    struct ext2_linux i_osd2;
-} __attribute__((packed));
-
-struct ext2_file_handle {
-    struct volume *part;
-    struct ext2_superblock sb;
-    int size;
-    struct ext2_inode root_inode;
-    struct ext2_inode inode;
-    uint64_t block_size;
-    uint32_t *alloc_map;
-};
-
-int ext2_check_signature(struct volume *part);
 bool ext2_get_guid(struct guid *guid, struct volume *part);
 char *ext2_get_label(struct volume *part);
 
-bool ext2_open(struct ext2_file_handle *ret, struct volume *part, const char *path);
-void ext2_read(struct ext2_file_handle *file, void *buf, uint64_t loc, uint64_t count);
-void ext2_close(struct ext2_file_handle *file);
+struct file_handle *ext2_open(struct volume *part, const char *path);
 
 #endif
diff --git a/common/fs/ext2.s2.c b/common/fs/ext2.s2.c
index d1379764..12b20c27 100644
--- a/common/fs/ext2.s2.c
+++ b/common/fs/ext2.s2.c
@@ -8,6 +8,121 @@
 #include <lib/print.h>
 #include <mm/pmm.h>
 
+/* Superblock Fields */
+struct ext2_superblock {
+    uint32_t s_inodes_count;
+    uint32_t s_blocks_count;
+    uint32_t s_r_blocks_count;
+    uint32_t s_free_blocks_count;
+    uint32_t s_free_inodes_count;
+    uint32_t s_first_data_block;
+    uint32_t s_log_block_size;
+    uint32_t s_log_frag_size;
+    uint32_t s_blocks_per_group;
+    uint32_t s_frags_per_group;
+    uint32_t s_inodes_per_group;
+    uint32_t s_mtime;
+    uint32_t s_wtime;
+
+    uint16_t s_mnt_count;
+    uint16_t s_max_mnt_count;
+    uint16_t s_magic;
+    uint16_t s_state;
+    uint16_t s_errors;
+    uint16_t s_minor_rev_level;
+
+    uint32_t s_lastcheck;
+    uint32_t s_checkinterval;
+    uint32_t s_creator_os;
+    uint32_t s_rev_level;
+    uint16_t s_def_resuid;
+    uint16_t s_def_gid;
+
+    // if version number >= 1, we have to use the ext2 extended superblock as well
+
+    /* Extended Superblock */
+    uint32_t s_first_ino;
+
+    uint16_t s_inode_size;
+    uint16_t s_block_group_nr;
+
+    uint32_t s_feature_compat;
+    uint32_t s_feature_incompat;
+    uint32_t s_feature_ro_compat;
+
+    uint64_t s_uuid[2];
+    uint8_t s_volume_name[16];
+
+    uint64_t s_last_mounted[8];
+
+    uint32_t compression_info;
+    uint8_t prealloc_blocks;
+    uint8_t prealloc_dir_blocks;
+    uint16_t reserved_gdt_blocks;
+    uint8_t journal_uuid[16];
+    uint32_t journal_inum;
+    uint32_t journal_dev;
+    uint32_t last_orphan;
+    uint32_t hash_seed[4];
+    uint8_t def_hash_version;
+    uint8_t jnl_backup_type;
+    uint16_t group_desc_size;
+    uint32_t default_mount_opts;
+    uint32_t first_meta_bg;
+    uint32_t mkfs_time;
+    uint32_t jnl_blocks[17];
+} __attribute__((packed));
+
+struct ext2_linux {
+    uint8_t  frag_num;
+    uint8_t  frag_size;
+
+    uint16_t reserved_16;
+    uint16_t user_id_high;
+    uint16_t group_id_high;
+
+    uint32_t reserved_32;
+} __attribute__((packed));
+
+struct ext2_inode {
+    uint16_t i_mode;
+    uint16_t i_uid;
+
+    uint32_t i_size;
+    uint32_t i_atime;
+    uint32_t i_ctime;
+    uint32_t i_mtime;
+    uint32_t i_dtime;
+
+    uint16_t i_gid;
+    uint16_t i_links_count;
+
+    uint32_t i_blocks_count;
+    uint32_t i_flags;
+    uint32_t i_osd1;
+    uint32_t i_blocks[15];
+    uint32_t i_generation;
+
+    /* EXT2 v >= 1.0 */
+    uint32_t i_eab;
+    uint32_t i_maj;
+
+    /* EXT2 vAll */
+    uint32_t i_frag_block;
+
+    struct ext2_linux i_osd2;
+} __attribute__((packed));
+
+struct ext2_file_handle {
+    struct volume *part;
+    struct ext2_superblock sb;
+    int size;
+    struct ext2_inode root_inode;
+    struct ext2_inode inode;
+    uint64_t block_size;
+    uint32_t *alloc_map;
+};
+
 /* Inode types */
 #define S_IFIFO  0x1000
 #define S_IFCHR  0x2000
@@ -314,19 +429,30 @@ out:
     return ret;
 }
 
-bool ext2_open(struct ext2_file_handle *ret, struct volume *part, const char *path) {
+static void ext2_read(struct file_handle *handle, void *buf, uint64_t loc, uint64_t count);
+static void ext2_close(struct file_handle *file);
+
+struct file_handle *ext2_open(struct volume *part, const char *path) {
+    struct ext2_file_handle *ret = ext_mem_alloc(sizeof(struct ext2_file_handle));
+
     ret->part = part;
 
     volume_read(ret->part, &ret->sb, 1024, sizeof(struct ext2_superblock));
 
     struct ext2_superblock *sb = &ret->sb;
 
+    if (sb->s_magic != EXT2_S_MAGIC) {
+        pmm_free(ret, sizeof(struct ext2_file_handle));
+        return NULL;
+    }
+
     if (sb->s_rev_level != 0 &&
         (sb->s_feature_incompat & EXT2_IF_COMPRESSION ||
          sb->s_feature_incompat & EXT2_IF_INLINE_DATA ||
          sb->s_feature_incompat & EXT2_FEATURE_INCOMPAT_META_BG)) {
         print("ext2: filesystem has unsupported features %x\n", sb->s_feature_incompat);
-        return false;
+        pmm_free(ret, sizeof(struct ext2_file_handle));
+        return NULL;
     }
 
     if (sb->s_rev_level != 0 && sb->s_feature_incompat & EXT2_IF_ENCRYPT) {
@@ -335,7 +461,8 @@ bool ext2_open(struct ext2_file_handle *ret, struct volume *part, const char *pa
 
     if (sb->s_state == EXT2_FS_UNRECOVERABLE_ERRORS) {
         print("ext2: unrecoverable errors found\n");
-        return false;
+        pmm_free(ret, sizeof(struct ext2_file_handle));
+        return NULL;
     }
 
     ret->block_size = ((uint64_t)1024 << ret->sb.s_log_block_size);
@@ -344,18 +471,23 @@ bool ext2_open(struct ext2_file_handle *ret, struct volume *part, const char *pa
 
     struct ext2_dir_entry entry;
 
-    if (!ext2_parse_dirent(&entry, ret, path))
-        return false;
+    if (!ext2_parse_dirent(&entry, ret, path)) {
+        pmm_free(ret, sizeof(struct ext2_file_handle));
+        return NULL;
+    }
 
     ext2_get_inode(&ret->inode, ret, entry.inode);
 
     while ((ret->inode.i_mode & FMT_MASK) != S_IFREG) {
         if ((ret->inode.i_mode & FMT_MASK) == S_IFLNK) {
-            if (!symlink_to_inode(&ret->inode, ret))
-                return false;
+            if (!symlink_to_inode(&ret->inode, ret)) {
+                pmm_free(ret, sizeof(struct ext2_file_handle));
+                return NULL;
+            }
         } else {
             print("ext2: Entity is not regular file nor symlink\n");
-            return false;
+            pmm_free(ret, sizeof(struct ext2_file_handle));
+            return NULL;
         }
     }
 
@@ -363,18 +495,32 @@ bool ext2_open(struct ext2_file_handle *ret, struct volume *part, const char *pa
 
     ret->alloc_map = create_alloc_map(ret, &ret->inode);
 
-    return true;
+    struct file_handle *handle = ext_mem_alloc(sizeof(struct file_handle));
+
+    handle->fd = ret;
+    handle->read = (void *)ext2_read;
+    handle->close = (void *)ext2_close;
+    handle->size = ret->size;
+    handle->vol = part;
+#if uefi == 1
+    handle->efi_part_handle = part->efi_part_handle;
+#endif
+
+    return handle;
 }
 
-void ext2_close(struct ext2_file_handle *file) {
-    if (file->alloc_map != NULL) {
-        pmm_free(file->alloc_map, file->inode.i_blocks_count * sizeof(uint32_t));
+static void ext2_close(struct file_handle *file) {
+    struct ext2_file_handle *f = file->fd;
+    if (f->alloc_map != NULL) {
+        pmm_free(f->alloc_map, f->inode.i_blocks_count * sizeof(uint32_t));
     }
-    pmm_free(file, sizeof(struct ext2_file_handle));
+    pmm_free(f, sizeof(struct ext2_file_handle));
+    pmm_free(file, sizeof(struct file_handle));
 }
 
-void ext2_read(struct ext2_file_handle *file, void *buf, uint64_t loc, uint64_t count) {
-    inode_read(buf, loc, count, &file->inode, file, file->alloc_map);
+static void ext2_read(struct file_handle *file, void *buf, uint64_t loc, uint64_t count) {
+    struct ext2_file_handle *f = file->fd;
+    inode_read(buf, loc, count, &f->inode, f, f->alloc_map);
 }
 
 static struct ext4_extent_header *ext4_find_leaf(struct ext4_extent_header *ext_block, uint32_t read_block, uint64_t block_size, struct volume *part) {
@@ -466,16 +612,6 @@ static int inode_read(void *buf, uint64_t loc, uint64_t count,
     return 0;
 }
 
-int ext2_check_signature(struct volume *part) {
-    struct ext2_superblock sb;
-    volume_read(part, &sb, 1024, sizeof(struct ext2_superblock));
-
-    if (sb.s_magic != EXT2_S_MAGIC)
-        return 0;
-
-    return 1;
-}
-
 bool ext2_get_guid(struct guid *guid, struct volume *part) {
     struct ext2_superblock sb;
     volume_read(part, &sb, 1024, sizeof(struct ext2_superblock));
diff --git a/common/fs/fat32.h b/common/fs/fat32.h
index 052bb2cc..3c9dee45 100644
--- a/common/fs/fat32.h
+++ b/common/fs/fat32.h
@@ -1,41 +1,11 @@
 #ifndef __FS__FAT32_H__
 #define __FS__FAT32_H__
 
-#include <stdint.h>
 #include <lib/part.h>
+#include <fs/file.h>
 
-struct fat32_context {
-    struct volume *part;
-    int type;
-    char label[12];
-    uint16_t bytes_per_sector;
-    uint8_t sectors_per_cluster;
-    uint16_t reserved_sectors;
-    uint8_t number_of_fats;
-    uint32_t hidden_sectors;
-    uint32_t sectors_per_fat;
-    uint32_t fat_start_lba;
-    uint32_t data_start_lba;
-    uint32_t root_directory_cluster;
-    uint16_t root_entries;
-    uint32_t root_start;
-    uint32_t root_size;
-};
-
-struct fat32_file_handle {
-    struct fat32_context context;
-    uint32_t first_cluster;
-    uint32_t size_bytes;
-    uint32_t size_clusters;
-    uint32_t *cluster_chain;
-    size_t chain_len;
-};
-
-int fat32_check_signature(struct volume *part);
 char *fat32_get_label(struct volume *part);
 
-bool fat32_open(struct fat32_file_handle *ret, struct volume *part, const char *path);
-void fat32_read(struct fat32_file_handle *file, void *buf, uint64_t loc, uint64_t count);
-void fat32_close(struct fat32_file_handle *file);
+struct file_handle *fat32_open(struct volume *part, const char *path);
 
 #endif
diff --git a/common/fs/fat32.s2.c b/common/fs/fat32.s2.c
index 1f75ff91..2f8563ff 100644
--- a/common/fs/fat32.s2.c
+++ b/common/fs/fat32.s2.c
@@ -12,6 +12,33 @@
 #define FAT32_ATTRIBUTE_SUBDIRECTORY 0x10
 #define FAT32_LFN_ATTRIBUTE 0x0F
 
+struct fat32_context {
+    struct volume *part;
+    int type;
+    char label[12];
+    uint16_t bytes_per_sector;
+    uint8_t sectors_per_cluster;
+    uint16_t reserved_sectors;
+    uint8_t number_of_fats;
+    uint32_t hidden_sectors;
+    uint32_t sectors_per_fat;
+    uint32_t fat_start_lba;
+    uint32_t data_start_lba;
+    uint32_t root_directory_cluster;
+    uint16_t root_entries;
+    uint32_t root_start;
+    uint32_t root_size;
+};
+
+struct fat32_file_handle {
+    struct fat32_context context;
+    uint32_t first_cluster;
+    uint32_t size_bytes;
+    uint32_t size_clusters;
+    uint32_t *cluster_chain;
+    size_t chain_len;
+};
+
 struct fat32_bpb {
     union {
         struct {
@@ -366,11 +393,6 @@ out:
     return ret;
 }
 
-int fat32_check_signature(struct volume *part) {
-    struct fat32_context context;
-    return fat32_init_context(&context, part) == 0;
-}
-
 char *fat32_get_label(struct volume *part) {
     struct fat32_context context;
     if (fat32_init_context(&context, part) != 0) {
@@ -388,13 +410,15 @@ char *fat32_get_label(struct volume *part) {
     return ret;
 }
 
-bool fat32_open(struct fat32_file_handle* ret, struct volume *part, const char* path) {
+static void fat32_read(struct file_handle *handle, void *buf, uint64_t loc, uint64_t count);
+static void fat32_close(struct file_handle *file);
+
+struct file_handle *fat32_open(struct volume *part, const char *path) {
     struct fat32_context context;
     int r = fat32_init_context(&context, part);
 
     if (r) {
-        print("fat32: context init failure (%d)\n", r);
-        return false;
+        return NULL;
     }
 
     struct fat32_directory_entry _current_directory;
@@ -444,13 +468,16 @@ bool fat32_open(struct fat32_file_handle* ret, struct volume *part, const char*
         }
 
         if ((r = fat32_open_in(&context, current_directory, &current_file, current_part)) != 0) {
-            return false;
+            return NULL;
         }
 
         if (expect_directory) {
             _current_directory = current_file;
             current_directory = &_current_directory;
         } else {
+            struct file_handle *handle = ext_mem_alloc(sizeof(struct file_handle));
+            struct fat32_file_handle *ret = ext_mem_alloc(sizeof(struct fat32_file_handle));
+
             ret->context = context;
             ret->first_cluster = current_file.cluster_num_low;
             if (context.type == 32)
@@ -458,16 +485,29 @@ bool fat32_open(struct fat32_file_handle* ret, struct volume *part, const char*
             ret->size_clusters = DIV_ROUNDUP(current_file.file_size_bytes, context.bytes_per_sector);
             ret->size_bytes = current_file.file_size_bytes;
             ret->cluster_chain = cache_cluster_chain(&context, ret->first_cluster, &ret->chain_len);
-            return true;
+
+            handle->fd = (void *)ret;
+            handle->read = (void *)fat32_read;
+            handle->close = (void *)fat32_close;
+            handle->size = ret->size_bytes;
+            handle->vol = part;
+#if uefi == 1
+            handle->efi_part_handle = part->efi_part_handle;
+#endif
+
+            return handle;
         }
     }
 }
 
-void fat32_read(struct fat32_file_handle* file, void* buf, uint64_t loc, uint64_t count) {
-    read_cluster_chain(&file->context, file->cluster_chain, buf, loc, count);
+static void fat32_read(struct file_handle *file, void *buf, uint64_t loc, uint64_t count) {
+    struct fat32_file_handle *f = file->fd;
+    read_cluster_chain(&f->context, f->cluster_chain, buf, loc, count);
 }
 
-void fat32_close(struct fat32_file_handle *file) {
-    pmm_free(file->cluster_chain, file->chain_len * sizeof(uint32_t));
-    pmm_free(file, sizeof(struct fat32_file_handle));
+static void fat32_close(struct file_handle *file) {
+    struct fat32_file_handle *f = file->fd;
+    pmm_free(f->cluster_chain, f->chain_len * sizeof(uint32_t));
+    pmm_free(f, sizeof(struct fat32_file_handle));
+    pmm_free(file, sizeof(struct file_handle));
 }
diff --git a/common/fs/file.s2.c b/common/fs/file.s2.c
index 723a91df..42d11068 100644
--- a/common/fs/file.s2.c
+++ b/common/fs/file.s2.c
@@ -14,133 +14,61 @@
 #include <pxe/tftp.h>
 
 char *fs_get_label(struct volume *part) {
-    if (fat32_check_signature(part)) {
-        return fat32_get_label(part);
+    char *ret;
+
+    if ((ret = fat32_get_label(part)) != NULL) {
+        return ret;
     }
-    if (ext2_check_signature(part)) {
-        return ext2_get_label(part);
+    if ((ret = ext2_get_label(part)) != NULL) {
+        return ret;
     }
 
     return NULL;
 }
 
 bool fs_get_guid(struct guid *guid, struct volume *part) {
-    if (echfs_check_signature(part)) {
-        return echfs_get_guid(guid, part);
+    if (echfs_get_guid(guid, part) == true) {
+        return true;
     }
-    if (ext2_check_signature(part)) {
-        return ext2_get_guid(guid, part);
+    if (ext2_get_guid(guid, part) == true) {
+        return true;
     }
 
     return false;
 }
 
 struct file_handle *fopen(struct volume *part, const char *filename) {
-    struct file_handle *ret = ext_mem_alloc(sizeof(struct file_handle));
-
-    ret->is_memfile = false;
-    ret->readall = false;
-
-    ret->vol = part;
-
     if (strlen(filename) + 2 > PATH_MAX) {
         panic(true, "fopen: Path too long");
     }
 
-    ret->path[0] = '/';
-
-    strcpy(ret->path + 1, filename);
+    struct file_handle *ret;
 
 #if bios == 1
     if (part->pxe) {
-        if (!tftp_open(ret, 0, 69, filename)) {
-            goto fail;
+        if ((ret = tftp_open(0, 69, filename)) == NULL) {
+            return NULL;
         }
         return ret;
     }
 #endif
 
-#if uefi == 1
-    ret->efi_part_handle = part->efi_part_handle;
-#endif
-
-    if (iso9660_check_signature(part)) {
-        struct iso9660_file_handle *fd = ext_mem_alloc(sizeof(struct iso9660_file_handle));
-
-        if (!iso9660_open(fd, part, filename)) {
-            goto fail;
-        }
-
-        ret->fd = (void *)fd;
-        ret->read = (void *)iso9660_read;
-        ret->close = (void *)iso9660_close;
-        ret->size = fd->size;
-
+    if ((ret = echfs_open(part, filename)) != NULL) {
         return ret;
     }
-
-    if (echfs_check_signature(part)) {
-        struct echfs_file_handle *fd = ext_mem_alloc(sizeof(struct echfs_file_handle));
-
-        if (!echfs_open(fd, part, filename)) {
-            goto fail;
-        }
-
-        ret->fd = (void *)fd;
-        ret->read = (void *)echfs_read;
-        ret->close = (void *)echfs_close;
-        ret->size = fd->dir_entry.size;
-
+    if ((ret = ext2_open(part, filename)) != NULL) {
         return ret;
     }
-
-    if (ext2_check_signature(part)) {
-        struct ext2_file_handle *fd = ext_mem_alloc(sizeof(struct ext2_file_handle));
-
-        if (!ext2_open(fd, part, filename)) {
-            goto fail;
-        }
-
-        ret->fd = (void *)fd;
-        ret->read = (void *)ext2_read;
-        ret->close = (void *)ext2_close;
-        ret->size = fd->size;
-
+    if ((ret = fat32_open(part, filename)) != NULL) {
         return ret;
     }
-
-    if (fat32_check_signature(part)) {
-        struct fat32_file_handle *fd = ext_mem_alloc(sizeof(struct fat32_file_handle));
-
-        if (!fat32_open(fd, part, filename)) {
-            goto fail;
-        }
-
-        ret->fd = (void *)fd;
-        ret->read = (void *)fat32_read;
-        ret->close = (void *)fat32_close;
-        ret->size = fd->size_bytes;
-
+    if ((ret = iso9660_open(part, filename)) != NULL) {
         return ret;
     }
-
-    if (ntfs_check_signature(part)) {
-        struct ntfs_file_handle *fd = ext_mem_alloc(sizeof(struct ntfs_file_handle));
-
-        if (!ntfs_open(fd, part, filename)) {
-            goto fail;
-        }
-
-        ret->fd = (void *)fd;
-        ret->read = (void *)ntfs_read;
-        ret->close = (void *)ntfs_close;
-        ret->size = fd->size_bytes;
-
+    if ((ret = ntfs_open(part, filename)) != NULL) {
         return ret;
     }
 
-fail:
-    pmm_free(ret, sizeof(struct file_handle));
     return NULL;
 }
 
@@ -149,17 +77,17 @@ void fclose(struct file_handle *fd) {
         if (fd->readall == false) {
             pmm_free(fd->fd, fd->size);
         }
+        pmm_free(fd, sizeof(struct file_handle));
     } else {
-        fd->close(fd->fd);
+        fd->close(fd);
     }
-    pmm_free(fd, sizeof(struct file_handle));
 }
 
 void fread(struct file_handle *fd, void *buf, uint64_t loc, uint64_t count) {
     if (fd->is_memfile) {
         memcpy(buf, fd->fd + loc, count);
     } else {
-        fd->read(fd->fd, buf, loc, count);
+        fd->read(fd, buf, loc, count);
     }
 }
 
@@ -173,7 +101,7 @@ void *freadall(struct file_handle *fd, uint32_t type) {
         return fd->fd;
     } else {
         void *ret = ext_mem_alloc_type(fd->size, type);
-        fd->read(fd->fd, ret, 0, fd->size);
+        fd->read(fd, ret, 0, fd->size);
         return ret;
     }
 }
diff --git a/common/fs/iso9660.h b/common/fs/iso9660.h
index 8f13eccc..acca7455 100644
--- a/common/fs/iso9660.h
+++ b/common/fs/iso9660.h
@@ -3,24 +3,8 @@
 
 #include <stdint.h>
 #include <lib/part.h>
+#include <fs/file.h>
 
-#define ISO9660_SECTOR_SIZE (2 << 10)
-
-struct iso9660_context {
-    struct volume *vol;
-    void* root;
-    uint32_t root_size;
-};
-
-struct iso9660_file_handle {
-    struct iso9660_context *context;
-    uint32_t LBA;
-    uint32_t size;
-};
-
-int iso9660_check_signature(struct volume *vol);
-bool iso9660_open(struct iso9660_file_handle *ret, struct volume *vol, const char *path);
-void iso9660_read(struct iso9660_file_handle *file, void *buf, uint64_t loc, uint64_t count);
-void iso9660_close(struct iso9660_file_handle *file);
+struct file_handle *iso9660_open(struct volume *vol, const char *path);
 
 #endif
diff --git a/common/fs/iso9660.s2.c b/common/fs/iso9660.s2.c
index ad5cf567..6a01bfce 100644
--- a/common/fs/iso9660.s2.c
+++ b/common/fs/iso9660.s2.c
@@ -3,6 +3,20 @@
 #include <lib/libc.h>
 #include <mm/pmm.h>
 
+#define ISO9660_SECTOR_SIZE (2 << 10)
+
+struct iso9660_context {
+    struct volume *vol;
+    void *root;
+    uint32_t root_size;
+};
+
+struct iso9660_file_handle {
+    struct iso9660_context *context;
+    uint32_t LBA;
+    uint32_t size;
+};
+
 #define ISO9660_FIRST_VOLUME_DESCRIPTOR 0x10
 #define ISO9660_VOLUME_DESCRIPTOR_SIZE ISO9660_SECTOR_SIZE
 #define ROCK_RIDGE_MAX_FILENAME 255
@@ -198,17 +212,20 @@ static struct iso9660_directory_entry *iso9660_find(void *buffer, uint32_t size,
     return NULL;
 }
 
+static void iso9660_read(struct file_handle *handle, void *buf, uint64_t loc, uint64_t count);
+static void iso9660_close(struct file_handle *file);
 
-// --- Public functions ---
-int iso9660_check_signature(struct volume *vol) {
+struct file_handle *iso9660_open(struct volume *vol, const char *path) {
     char buf[6];
     const uint64_t signature = ISO9660_FIRST_VOLUME_DESCRIPTOR * ISO9660_SECTOR_SIZE + 1;
     volume_read(vol, buf, signature, 5);
     buf[5] = '\0';
-    return !strcmp(buf, "CD001");
-}
+    if (strcmp(buf, "CD001") != 0) {
+        return NULL;
+    }
+
+    struct iso9660_file_handle *ret = ext_mem_alloc(sizeof(struct iso9660_file_handle));
 
-bool iso9660_open(struct iso9660_file_handle *ret, struct volume *vol, const char *path) {
     ret->context = iso9660_get_context(vol);
 
     while (*path == '/')
@@ -230,8 +247,10 @@ bool iso9660_open(struct iso9660_file_handle *ret, struct volume *vol, const cha
         *aux = '\0';
 
         struct iso9660_directory_entry *entry = iso9660_find(current, current_size, filename);
-        if (!entry)
-            return false;    // Not found :(
+        if (!entry) {
+            pmm_free(ret, sizeof(struct iso9660_file_handle));
+            return NULL;    // Not found :(
+        }
 
         next_sector = entry->extent.little;
         next_size = entry->extent_size.little;
@@ -253,13 +272,27 @@ bool iso9660_open(struct iso9660_file_handle *ret, struct volume *vol, const cha
 
     ret->LBA = next_sector;
     ret->size = next_size;
-    return true;
+
+    struct file_handle *handle = ext_mem_alloc(sizeof(struct file_handle));
+
+    handle->fd = ret;
+    handle->read = (void *)iso9660_read;
+    handle->close = (void *)iso9660_close;
+    handle->size = ret->size;
+    handle->vol = vol;
+#if uefi == 1
+    handle->efi_part_handle = vol->efi_part_handle;
+#endif
+
+    return handle;
 }
 
-void iso9660_read(struct iso9660_file_handle *file, void *buf, uint64_t loc, uint64_t count) {
-    volume_read(file->context->vol, buf, file->LBA * ISO9660_SECTOR_SIZE + loc, count);
+static void iso9660_read(struct file_handle *file, void *buf, uint64_t loc, uint64_t count) {
+    struct iso9660_file_handle *f = file->fd;
+    volume_read(f->context->vol, buf, f->LBA * ISO9660_SECTOR_SIZE + loc, count);
 }
 
-void iso9660_close(struct iso9660_file_handle *file) {
-    pmm_free(file, sizeof(struct iso9660_file_handle));
+static void iso9660_close(struct file_handle *file) {
+    pmm_free(file->fd, sizeof(struct iso9660_file_handle));
+    pmm_free(file, sizeof(struct file_handle));
 }
diff --git a/common/fs/ntfs.h b/common/fs/ntfs.h
index 5eb1d3f5..a508ff4e 100644
--- a/common/fs/ntfs.h
+++ b/common/fs/ntfs.h
@@ -1,72 +1,9 @@
 #ifndef __FS__NTFS_H__
 #define __FS__NTFS_H__
 
-#include <stdint.h>
-#include <stddef.h>
-#include <stdbool.h>
 #include <lib/part.h>
-#include <lib/blib.h>
+#include <fs/file.h>
 
-struct ntfs_bpb {
-    uint8_t jump[3];
-    char oem[8];
-    uint16_t bytes_per_sector;
-    uint8_t sectors_per_cluster;
-    uint16_t reserved_sectors;
-    uint8_t fats_count;
-    uint16_t directory_entries_count;
-    uint16_t sector_totals;
-    uint8_t media_descriptor_type;
-    uint16_t sectors_per_fat_16;
-    uint16_t sectors_per_track;
-    uint16_t heads_count;
-    uint32_t hidden_sectors_count;
-    uint32_t large_sectors_count;
-    
-    // ntfs 
-    uint32_t sectors_per_fat_32;
-    uint64_t sectors_count_64;
-    uint64_t mft_cluster;
-} __attribute__((packed));
+struct file_handle *ntfs_open(struct volume *part, const char *path);
 
-struct ntfs_file_handle {
-    struct volume *part;
-
-    struct ntfs_bpb bpb;
-
-    // file record sizes
-    uint64_t file_record_size;
-    uint64_t sectors_per_file_record;
-
-    // MFT info, the offset and its runlist
-    uint64_t mft_offset;
-    uint8_t mft_run_list[256];
-
-    // the runlist of the open file/directory
-    uint8_t run_list[128];
-
-    // The resident index, only for directories,
-    // could be at the same time as a runlist
-    uint8_t resident_index_size;
-    
-    // the resident data 
-    uint8_t resident_data_size;
-
-    // we are using a union just for having different names, these
-    // won't have need to be used at the same time
-    union {
-        uint8_t resident_index[1024];
-        uint8_t resident_data[1024];
-    };
-
-    // info about the current file
-    uint32_t size_bytes;
-};
-
-int ntfs_check_signature(struct volume *part);
-
-bool ntfs_open(struct ntfs_file_handle *ret, struct volume *part, const char *path);
-int ntfs_read(struct ntfs_file_handle *file, void *buf, uint64_t loc, uint64_t count);
-void ntfs_close(struct ntfs_file_handle *file);
-
-#endif
\ No newline at end of file
+#endif
diff --git a/common/fs/ntfs.s2.c b/common/fs/ntfs.s2.c
index 74877e14..b187c4db 100644
--- a/common/fs/ntfs.s2.c
+++ b/common/fs/ntfs.s2.c
@@ -1,12 +1,71 @@
+#include <stdint.h>
+#include <stddef.h>
+#include <stdbool.h>
 #include <fs/ntfs.h>
 #include <mm/pmm.h>
 #include <lib/print.h>
-
-#include <stdbool.h>
+#include <lib/libc.h>
+#include <lib/blib.h>
 
 // created using documentation from:
 //  https://dubeyko.com/development/FileSystems/NTFS/ntfsdoc.pdf
 
+struct ntfs_bpb {
+    uint8_t jump[3];
+    char oem[8];
+    uint16_t bytes_per_sector;
+    uint8_t sectors_per_cluster;
+    uint16_t reserved_sectors;
+    uint8_t fats_count;
+    uint16_t directory_entries_count;
+    uint16_t sector_totals;
+    uint8_t media_descriptor_type;
+    uint16_t sectors_per_fat_16;
+    uint16_t sectors_per_track;
+    uint16_t heads_count;
+    uint32_t hidden_sectors_count;
+    uint32_t large_sectors_count;
+
+    // ntfs
+    uint32_t sectors_per_fat_32;
+    uint64_t sectors_count_64;
+    uint64_t mft_cluster;
+} __attribute__((packed));
+
+struct ntfs_file_handle {
+    struct volume *part;
+
+    struct ntfs_bpb bpb;
+
+    // file record sizes
+    uint64_t file_record_size;
+    uint64_t sectors_per_file_record;
+
+    // MFT info, the offset and its runlist
+    uint64_t mft_offset;
+    uint8_t mft_run_list[256];
+
+    // the runlist of the open file/directory
+    uint8_t run_list[128];
+
+    // The resident index, only for directories,
+    // could be at the same time as a runlist
+    uint8_t resident_index_size;
+
+    // the resident data
+    uint8_t resident_data_size;
+
+    // we are using a union just for having different names, these
+    // won't have need to be used at the same time
+    union {
+        uint8_t resident_index[1024];
+        uint8_t resident_data[1024];
+    };
+
+    // info about the current file
+    uint32_t size_bytes;
+};
+
 // This is the total size of a file record, including the attributes
 // TODO: calculate this
 #define MIN_FILE_RECORD_SIZE 1024
@@ -121,36 +180,6 @@ struct index_entry {
     uint16_t name[];
 } __attribute__((packed));
 
-int ntfs_check_signature(struct volume *part) {
-    struct ntfs_bpb bpb;
-    if (!volume_read(part, &bpb, 0, sizeof(bpb))) {
-        return 0;
-    }
-
-    //
-    // validate the bpb
-    //
-
-    if (strncmp(bpb.oem, "NTFS    ", SIZEOF_ARRAY(bpb.oem))) {
-        return 0;
-    }
-
-    if (bpb.sector_totals != 0) {
-        return 0;
-    }
-
-    if (bpb.large_sectors_count != 0) {
-        return 0;
-    }
-
-    if (bpb.sectors_count_64 == 0) {
-        return 0;
-    }
-
-    // this is a valid ntfs sector
-    return 1;
-}
-
 // the temp buffer is used for storing dirs and alike
 // in memory, because limine only has allocate without
 // free we are going to allocate it once globally and just
@@ -159,6 +188,10 @@ static uint8_t *dir_buffer = NULL;
 static size_t dir_buffer_size = 0;
 static size_t dir_buffer_cap = 0;
 
+// XXX ugly hack due to broken layering
+static int ntfs_read(struct file_handle *handle, void *buf, uint64_t loc, uint64_t count);
+static void ntfs_close(struct file_handle *file);
+
 /**
  * Get an attribute from the given file record
  */
@@ -377,10 +410,14 @@ static bool ntfs_read_directory(struct ntfs_file_handle *handle, uint64_t mft_re
         dir_buffer_size = dir_size;
 
         // read the directory
-        if (ntfs_read(handle, dir_buffer, 0, dir_size)) {
+        // XXX ugly hack due to broken layering
+        {
+        struct file_handle h = { .fd = handle };
+        if (ntfs_read(&h, dir_buffer, 0, dir_size)) {
             print("NTFS: EOF before reading directory fully...\n");
             return false;
         }
+        }
     } else {
         // if no runlist then empty the runlist
         memset(handle->run_list, 0, sizeof(handle->run_list));
@@ -546,14 +583,41 @@ static bool ntfs_find_file_in_directory(struct ntfs_file_handle *handle, const c
     return false;
 }
 
-bool ntfs_open(struct ntfs_file_handle *ret, struct volume *part, const char *path) {
+struct file_handle *ntfs_open(struct volume *part, const char *path) {
+    struct ntfs_file_handle *ret = ext_mem_alloc(sizeof(struct ntfs_file_handle));
+
     // save the part
     ret->part = part;
 
     // start by reading the bpb so we can access it later on
     if (!volume_read(part, &ret->bpb, 0, sizeof(ret->bpb))) {
         print("NTFS: Failed to read the BPB\n");
-        return false;
+        pmm_free(ret, sizeof(struct ntfs_file_handle));
+        return NULL;
+    }
+
+    //
+    // validate the bpb
+    //
+
+    if (strncmp(ret->bpb.oem, "NTFS    ", SIZEOF_ARRAY(ret->bpb.oem))) {
+        pmm_free(ret, sizeof(struct ntfs_file_handle));
+        return NULL;
+    }
+
+    if (ret->bpb.sector_totals != 0) {
+        pmm_free(ret, sizeof(struct ntfs_file_handle));
+        return NULL;
+    }
+
+    if (ret->bpb.large_sectors_count != 0) {
+        pmm_free(ret, sizeof(struct ntfs_file_handle));
+        return NULL;
+    }
+
+    if (ret->bpb.sectors_count_64 == 0) {
+        pmm_free(ret, sizeof(struct ntfs_file_handle));
+        return NULL;
     }
 
     // in NTFS sector size can be 512 to 4096 bytes, file records are
@@ -570,13 +634,15 @@ bool ntfs_open(struct ntfs_file_handle *ret, struct volume *part, const char *pa
     }
     if (ret->file_record_size != MIN_FILE_RECORD_SIZE) {
         print("NTFS: TODO: support file record size which is not 1024 bytes\n");
-        return false;
+        pmm_free(ret, sizeof(struct ntfs_file_handle));
+        return NULL;
     }
 
     // now prepare the root directory so we can search for
     // the rest of the stuff
     if (!ntfs_read_root(ret)) {
-        return false;
+        pmm_free(ret, sizeof(struct ntfs_file_handle));
+        return NULL;
     }
 
     // iterate the directories to find the entry
@@ -590,8 +656,10 @@ bool ntfs_open(struct ntfs_file_handle *ret, struct volume *part, const char *pa
 
         // find the file in the directory
         entry = NULL;
-        if (!ntfs_find_file_in_directory(ret, current_path, &entry))
-            return false;
+        if (!ntfs_find_file_in_directory(ret, current_path, &entry)) {
+            pmm_free(ret, sizeof(struct ntfs_file_handle));
+            return NULL;
+        }
 
         size_t filename_len = entry->name_length;
 
@@ -604,14 +672,16 @@ bool ntfs_open(struct ntfs_file_handle *ret, struct volume *part, const char *pa
             // get its runlist...
             if (!ntfs_get_file_record(ret, entry->mft_record, file_record_buffer)) {
                 print("NTFS: Failed to get file record of file\n");
-                return false;
+                pmm_free(ret, sizeof(struct ntfs_file_handle));
+                return NULL;
             }
 
             // get the file attribute
             uint8_t *attr_ptr = NULL;
             if (!ntfs_get_file_record_attr(file_record_buffer, FR_ATTRIBUTE_DATA, &attr_ptr)) {
                 print("NTFS: File record missing DATA attribute\n");
-                return false;
+                pmm_free(ret, sizeof(struct ntfs_file_handle));
+                return NULL;
             }
             struct file_record_attr_header *attr_hdr = (struct file_record_attr_header *)attr_ptr;
 
@@ -625,11 +695,13 @@ bool ntfs_open(struct ntfs_file_handle *ret, struct volume *part, const char *pa
                 // verify the attr and run list are in the buffer
                 if ((uint8_t *)attr + sizeof(*attr) > file_record_buffer + sizeof(file_record_buffer)) {
                     print("NTFS: File record attribute is outside of file record\n");
-                    return false;
+                    pmm_free(ret, sizeof(struct ntfs_file_handle));
+                    return NULL;
                 }
                 if ((uint8_t *)attr + attr->run_offset + 256 > file_record_buffer + sizeof(file_record_buffer)) {
                     print("NTFS: Run list is outside of file record\n");
-                    return false;
+                    pmm_free(ret, sizeof(struct ntfs_file_handle));
+                    return NULL;
                 }
 
                 // save the run list
@@ -640,20 +712,32 @@ bool ntfs_open(struct ntfs_file_handle *ret, struct volume *part, const char *pa
 
                 if (attr->info_length > sizeof(ret->resident_data)) {
                     print("NTFS: Resident data too big\n");
-                    return false;
+                    pmm_free(ret, sizeof(struct ntfs_file_handle));
+                    return NULL;
                 }
 
                 ret->resident_data_size = attr->info_length;
                 memcpy(ret->resident_data, attr + 1, attr->info_length);
             }
 
-            return true;
+            struct file_handle *handle = ext_mem_alloc(sizeof(struct file_handle));
 
+            handle->fd = ret;
+            handle->read = (void *)ntfs_read;
+            handle->close = (void *)ntfs_close;
+            handle->size = ret->size_bytes;
+            handle->vol = part;
+#if uefi == 1
+            handle->efi_part_handle = part->efi_part_handle;
+#endif
+
+            return handle;
         } else {
             // read the directory
             if (!ntfs_read_directory(ret, entry->mft_record, file_record_buffer)) {
                 print("NTFS: Failed to read directory\n");
-                return false;
+                pmm_free(ret, sizeof(struct ntfs_file_handle));
+                return NULL;
             }
 
             // next path element
@@ -665,7 +749,9 @@ bool ntfs_open(struct ntfs_file_handle *ret, struct volume *part, const char *pa
     __builtin_unreachable();
 }
 
-int ntfs_read(struct ntfs_file_handle *file, void *buf, uint64_t loc, uint64_t count) {
+static int ntfs_read(struct file_handle *handle, void *buf, uint64_t loc, uint64_t count) {
+    struct ntfs_file_handle *file = handle->fd;
+
     // get the runlist
     uint8_t *runlist = file->run_list;
 
@@ -737,6 +823,7 @@ int ntfs_read(struct ntfs_file_handle *file, void *buf, uint64_t loc, uint64_t c
     return count != 0;
 }
 
-void ntfs_close(struct ntfs_file_handle *file) {
-    pmm_free(file, sizeof(struct ntfs_file_handle));
+static void ntfs_close(struct file_handle *file) {
+    pmm_free(file->fd, sizeof(struct ntfs_file_handle));
+    pmm_free(file, sizeof(struct file_handle));
 }
diff --git a/common/lib/config.c b/common/lib/config.c
index ca70e631..4e3f329e 100644
--- a/common/lib/config.c
+++ b/common/lib/config.c
@@ -40,15 +40,15 @@ int init_config_disk(struct volume *part) {
 
 #if bios == 1
 int init_config_pxe(void) {
-    struct file_handle f;
-    if (tftp_open(&f, 0, 69, "limine.cfg")) {
+    struct file_handle *f;
+    if ((f = tftp_open(0, 69, "limine.cfg")) == NULL) {
         return -1;
     }
 
-    size_t config_size = f.size + 2;
+    size_t config_size = f->size + 2;
     config_addr = ext_mem_alloc(config_size);
 
-    fread(&f, config_addr, 0, f.size);
+    fread(f, config_addr, 0, f->size);
 
     return init_config(config_size);
 }
diff --git a/common/lib/uri.c b/common/lib/uri.c
index 3f677224..40861f1c 100644
--- a/common/lib/uri.c
+++ b/common/lib/uri.c
@@ -162,8 +162,8 @@ static struct file_handle *uri_tftp_dispatch(char *root, char *path) {
         }
     }
 
-    struct file_handle *ret = ext_mem_alloc(sizeof(struct file_handle));
-    if (!tftp_open(ret, ip, 69, path)) {
+    struct file_handle *ret;
+    if ((ret = tftp_open(ip, 69, path)) == NULL) {
         return NULL;
     }
 
diff --git a/common/pxe/tftp.h b/common/pxe/tftp.h
index 169f2819..5fae5f0c 100644
--- a/common/pxe/tftp.h
+++ b/common/pxe/tftp.h
@@ -38,7 +38,7 @@ struct pxenv_get_file_size {
 #define TFTP_CLOSE 0x21
 
 //server_ip and server_port can be 0 for default
-bool tftp_open(struct file_handle *handle, uint32_t server_ip, uint16_t server_port, const char *name);
+struct file_handle *tftp_open(uint32_t server_ip, uint16_t server_port, const char *name);
 
 uint32_t get_boot_server_info(void);
 
diff --git a/common/pxe/tftp.s2.c b/common/pxe/tftp.s2.c
index d4f61aaf..25b00c17 100644
--- a/common/pxe/tftp.s2.c
+++ b/common/pxe/tftp.s2.c
@@ -16,7 +16,7 @@ uint32_t get_boot_server_info(void) {
     return ph->sip;
 }
 
-bool tftp_open(struct file_handle *handle, uint32_t server_ip, uint16_t server_port, const char *name) {
+struct file_handle *tftp_open(uint32_t server_ip, uint16_t server_port, const char *name) {
     int ret = 0;
 
     if (!server_ip) {
@@ -26,7 +26,7 @@ bool tftp_open(struct file_handle *handle, uint32_t server_ip, uint16_t server_p
     struct PXENV_UNDI_GET_INFORMATION undi_info = { 0 };
     ret = pxe_call(UNDI_GET_INFORMATION, ((uint16_t)rm_seg(&undi_info)), (uint16_t)rm_off(&undi_info));
     if (ret) {
-        return false;
+        return NULL;
     }
 
     //TODO figure out a more proper way to do this.
@@ -39,9 +39,11 @@ bool tftp_open(struct file_handle *handle, uint32_t server_ip, uint16_t server_p
     strcpy((char*)fsize.name, name);
     ret = pxe_call(TFTP_GET_FILE_SIZE, ((uint16_t)rm_seg(&fsize)), (uint16_t)rm_off(&fsize));
     if (ret) {
-        return false;
+        return NULL;
     }
 
+    struct file_handle *handle = ext_mem_alloc(sizeof(struct file_handle));
+
     handle->size = fsize.file_size;
     handle->is_memfile = true;
 
@@ -60,7 +62,8 @@ bool tftp_open(struct file_handle *handle, uint32_t server_ip, uint16_t server_p
     ret = pxe_call(TFTP_OPEN, ((uint16_t)rm_seg(&open)), (uint16_t)rm_off(&open));
     if (ret) {
         print("tftp: Failed to open file %x or bad packet size", open.status);
-        return false;
+        pmm_free(handle, sizeof(struct file_handle));
+        return NULL;
     }
 
     mtu = open.packet_size;
@@ -100,7 +103,7 @@ bool tftp_open(struct file_handle *handle, uint32_t server_ip, uint16_t server_p
 
     pmm_free(buf, mtu);
 
-    return true;
+    return handle;
 }
 
 #endif
tab: 248 wrap: offon