refactor ext2 map_inode_block_index_to_global_block_number

2 files changed, 62 insertions, 26 deletions

diff --git a/kernel/src/filesystem/ext2/filesystem.cpp b/kernel/src/filesystem/ext2/filesystem.cpp
index aaa50c7..0f34cfe 100644
--- a/kernel/src/filesystem/ext2/filesystem.cpp
+++ b/kernel/src/filesystem/ext2/filesystem.cpp
@@ -13,6 +13,7 @@
 #include <cstddef>
 #include <cstdint>
 #include <string_view>
+#include <unistd.h>
 
 namespace kernel::filesystem::ext2
 {
@@ -113,7 +114,7 @@ namespace kernel::filesystem::ext2
   }
 
   auto filesystem::map_inode_block_index_to_global_block_number(uint32_t inode_block_index, inode_data data) const
-      -> uint32_t
+      -> ssize_t
   {
     if (inode_block_index < constants::direct_block_count)
     {
@@ -121,9 +122,7 @@ namespace kernel::filesystem::ext2
     }
     inode_block_index -= constants::direct_block_count;
 
-    auto const block_size = get_block_size();
-    auto const numbers_per_block = block_size / sizeof(uint32_t);
-
+    auto const numbers_per_block = block_numbers_per_block();
     auto const block_numbers_per_singly_indirect_block = numbers_per_block;
     auto const block_numbers_per_doubly_indirect_block = numbers_per_block * block_numbers_per_singly_indirect_block;
     auto const block_numbers_per_triply_indirect_block = numbers_per_block * block_numbers_per_doubly_indirect_block;
@@ -131,44 +130,76 @@ namespace kernel::filesystem::ext2
     if (inode_block_index < block_numbers_per_singly_indirect_block)
     {
       auto const singly_indirect_block_number = data.block.at(constants::singly_indirect_block_index);
-      return read_block_number_at_index(singly_indirect_block_number, inode_block_index);
+      return read_singly_indirect_block_number(singly_indirect_block_number, inode_block_index);
     }
     inode_block_index -= block_numbers_per_singly_indirect_block;
 
     if (inode_block_index < block_numbers_per_doubly_indirect_block)
     {
       auto const doubly_indirect_block_number = data.block.at(constants::doubly_indirect_block_index);
-      auto const singly_indirect_block_index_in_doubly_indirect_block =
-          inode_block_index / block_numbers_per_singly_indirect_block;
-      auto const singly_indirect_block_number = read_block_number_at_index(
-          doubly_indirect_block_number, singly_indirect_block_index_in_doubly_indirect_block);
-
-      auto const block_index_in_singly_indirect_block = inode_block_index % block_numbers_per_singly_indirect_block;
-      return read_block_number_at_index(singly_indirect_block_number, block_index_in_singly_indirect_block);
+      return read_doubly_indirect_block_number(doubly_indirect_block_number, inode_block_index);
     }
     inode_block_index -= block_numbers_per_doubly_indirect_block;
 
     if (inode_block_index < block_numbers_per_triply_indirect_block)
     {
       auto const triply_indirect_block_number = data.block.at(constants::triply_indirect_block_index);
-      auto const doubly_indirect_block_index_in_triply_indirect_block =
-          inode_block_index / block_numbers_per_doubly_indirect_block;
-      auto const doubly_indirect_block_number = read_block_number_at_index(
-          triply_indirect_block_number, doubly_indirect_block_index_in_triply_indirect_block);
+      return read_triply_indirect_block_number(triply_indirect_block_number, inode_block_index);
+    }
+
+    return -1;
+  }
+
+  auto filesystem::read_singly_indirect_block_number(uint32_t singly_indirect_block_number,
+                                                     uint32_t block_index_in_singly_indirect_block) const -> uint32_t
+  {
+    if (singly_indirect_block_number == 0)
+    {
+      return 0;
+    }
+    return read_block_number_at_index(singly_indirect_block_number, block_index_in_singly_indirect_block);
+  }
+
+  auto filesystem::read_doubly_indirect_block_number(uint32_t doubly_indirect_block_number,
+                                                     uint32_t block_index_in_doubly_indirect_block) const -> uint32_t
+  {
+    auto const block_numbers_per_singly_indirect_block = block_numbers_per_block();
 
-      auto const remaining_block_numbers = inode_block_index % block_numbers_per_doubly_indirect_block;
+    if (doubly_indirect_block_number == 0)
+    {
+      return 0;
+    }
+
+    auto const singly_indirect_block_index_in_doubly_indirect_block =
+        block_index_in_doubly_indirect_block / block_numbers_per_singly_indirect_block;
+    auto const block_index_in_singly_indirect_block =
+        block_index_in_doubly_indirect_block % block_numbers_per_singly_indirect_block;
+
+    auto const singly_indirect_block_number =
+        read_block_number_at_index(doubly_indirect_block_number, singly_indirect_block_index_in_doubly_indirect_block);
 
-      auto const singly_indirect_block_index_in_doubly_indirect_block =
-          remaining_block_numbers / block_numbers_per_singly_indirect_block;
-      auto const singly_indirect_block_number = read_block_number_at_index(
-          doubly_indirect_block_number, singly_indirect_block_index_in_doubly_indirect_block);
+    return read_singly_indirect_block_number(singly_indirect_block_number, block_index_in_singly_indirect_block);
+  }
+
+  auto filesystem::read_triply_indirect_block_number(uint32_t triply_indirect_block_number,
+                                                     uint32_t block_index_in_triply_indirect_block) const -> uint32_t
+  {
+    auto const block_numbers_per_doubly_indirect_block = block_numbers_per_block() * block_numbers_per_block();
 
-      auto const block_index_in_singly_indirect_block =
-          remaining_block_numbers % block_numbers_per_singly_indirect_block;
-      return read_block_number_at_index(singly_indirect_block_number, block_index_in_singly_indirect_block);
+    if (triply_indirect_block_number == 0)
+    {
+      return 0;
     }
 
-    return 0;  // TODO BA-FS26 really correct??
+    auto const doubly_indirect_block_index_in_triply_indirect_block =
+        block_index_in_triply_indirect_block / block_numbers_per_doubly_indirect_block;
+    auto const block_index_in_doubly_indirect_block =
+        block_index_in_triply_indirect_block % block_numbers_per_doubly_indirect_block;
+
+    auto const doubly_indirect_block_number =
+        read_block_number_at_index(triply_indirect_block_number, doubly_indirect_block_index_in_triply_indirect_block);
+
+    return read_doubly_indirect_block_number(doubly_indirect_block_number, block_index_in_doubly_indirect_block);
   }
 
   auto filesystem::read_block_number_at_index(uint32_t block_number, uint32_t index) const -> uint32_t
@@ -182,6 +213,11 @@ namespace kernel::filesystem::ext2
     return block_number_buffer;
   }
 
+  auto filesystem::block_numbers_per_block() const -> uint32_t
+  {
+    return get_block_size() / sizeof(uint32_t);
+  }
+
   auto filesystem::get_block_size() const -> size_t
   {
     return constants::base_block_size << m_superblock.log_block_size;
diff --git a/kernel/src/filesystem/ext2/filesystem.tests.cpp b/kernel/src/filesystem/ext2/filesystem.tests.cpp
index 31c4c29..8341070 100644
--- a/kernel/src/filesystem/ext2/filesystem.tests.cpp
+++ b/kernel/src/filesystem/ext2/filesystem.tests.cpp
@@ -133,7 +133,7 @@ SCENARIO("Ext2 block mapping includes direct and all indirect levels", "[filesys
     THEN("mapping returns zero for out-of-range indexes")
     {
       auto const beyond_triply = triply_start + numbers_per_block * numbers_per_block * numbers_per_block;
-      REQUIRE(fs.map_inode_block_index_to_global_block_number(beyond_triply, inode_data) == 0);
+      REQUIRE(fs.map_inode_block_index_to_global_block_number(beyond_triply, inode_data) == -1);
     }
   }
 }