Merge branch 'refactor-ext2-inode-size' into 'develop-BA-FS26'

Determine ext2 inode size depending on revision level, add const to several methods

See merge request teachos/kernel!34

7 files changed, 152 insertions, 22 deletions

diff --git a/kernel/include/kernel/filesystem/ext2/filesystem.hpp b/kernel/include/kernel/filesystem/ext2/filesystem.hpp
index d22433f..18ef372 100644
--- a/kernel/include/kernel/filesystem/ext2/filesystem.hpp
+++ b/kernel/include/kernel/filesystem/ext2/filesystem.hpp
@@ -25,6 +25,9 @@ namespace kernel::filesystem::ext2
     constexpr size_t inline superblock_offset = base_block_size;
     constexpr uint16_t inline magic_number = 0xEF53;
 
+    constexpr uint32_t inline good_old_revision = 0;
+    constexpr uint32_t inline dynamic_revision = 1;
+
     constexpr uint32_t inline root_inode_number = 2;
 
     constexpr size_t inline direct_block_count = 12;
@@ -64,7 +67,13 @@ namespace kernel::filesystem::ext2
     @brief Gets the size of a block in the filesystem.
     @return The size of a block in bytes.
     */
-    auto get_block_size() -> size_t;
+    [[nodiscard]] auto get_block_size() const -> size_t;
+
+    /**
+    @brief Gets the revision level of the filesystem.
+    @return The revision level.
+    */
+    [[nodiscard]] auto get_revision_level() const -> size_t;
 
     /**
     @brief Maps an inode block index to a global block number.
@@ -72,14 +81,15 @@ namespace kernel::filesystem::ext2
     @param data The inode data.
     @return The global block number.
     */
-    auto map_inode_block_index_to_global_block_number(uint32_t inode_block_index, inode_data data) -> uint32_t;
+    [[nodiscard]] auto map_inode_block_index_to_global_block_number(uint32_t inode_block_index, inode_data data) const
+        -> uint32_t;
 
   private:
-    auto read_inode(uint32_t inode_number) -> kstd::shared_ptr<kernel::filesystem::ext2::inode>;
-    auto read_block_number_at_index(uint32_t block_number, uint32_t index) -> uint32_t;
+    [[nodiscard]] auto read_inode(uint32_t inode_number) const -> kstd::shared_ptr<kernel::filesystem::ext2::inode>;
+    [[nodiscard]] auto read_block_number_at_index(uint32_t block_number, uint32_t index) const -> uint32_t;
 
-    auto get_inode_size() -> size_t;
-    auto get_inode_block_count(inode_data const & data) -> uint32_t;
+    [[nodiscard]] auto get_inode_size() const -> size_t;
+    [[nodiscard]] auto get_inode_block_count(inode_data const & data) const -> uint32_t;
 
     superblock m_superblock{};
     kstd::vector<block_group_descriptor> m_block_group_descriptors;
diff --git a/kernel/include/kernel/filesystem/ext2/inode.hpp b/kernel/include/kernel/filesystem/ext2/inode.hpp
index b8f892a..000a5d8 100644
--- a/kernel/include/kernel/filesystem/ext2/inode.hpp
+++ b/kernel/include/kernel/filesystem/ext2/inode.hpp
@@ -20,7 +20,7 @@ namespace kernel::filesystem::ext2
   {
     uint16_t mode;
     uint16_t uid;
-    uint32_t size;  // TODO BA-FS26 signed?
+    uint32_t size;
     uint32_t atime;
     uint32_t ctime;
     uint32_t mtime;
@@ -45,7 +45,7 @@ namespace kernel::filesystem::ext2
     @param fs The ext2 filesystem that this inode belongs to.
     @param data The data associated with this inode, read from the disk.
     */
-    explicit inode(filesystem * fs, inode_data const & data);
+    explicit inode(filesystem const * fs, inode_data const & data);
 
     /**
     @brief Reads from the ext2 inode into a @p buffer, starting at the specified @p offset and for a given @p size.
@@ -90,8 +90,14 @@ namespace kernel::filesystem::ext2
      */
     [[nodiscard]] auto is_symbolic_link() const -> bool override;
 
+    /**
+    @brief Get the size of the file represented by this inode.
+    @return The size of the file in bytes.
+     */
+    [[nodiscard]] auto get_size() const -> size_t;
+
   private:
-    filesystem * m_filesystem;
+    filesystem const * m_filesystem;
     inode_data m_data{};
   };
 }  // namespace kernel::filesystem::ext2
diff --git a/kernel/include/kernel/test_support/filesystem/ext2.hpp b/kernel/include/kernel/test_support/filesystem/ext2.hpp
index 107e5a4..18cef1c 100644
--- a/kernel/include/kernel/test_support/filesystem/ext2.hpp
+++ b/kernel/include/kernel/test_support/filesystem/ext2.hpp
@@ -1,6 +1,7 @@
 #ifndef TEACHOS_KERNEL_TEST_SUPPORT_FILESYSTEM_EXT2_HPP
 #define TEACHOS_KERNEL_TEST_SUPPORT_FILESYSTEM_EXT2_HPP
 
+#include <kernel/filesystem/ext2/superblock.hpp>
 #include <kernel/test_support/devices/block_device.hpp>
 
 #include <cstddef>
@@ -12,6 +13,9 @@ namespace kernel::tests::filesystem::ext2
       -> void;
   auto write_u32(kernel::tests::devices::block_device & device, size_t offset, uint32_t value) -> void;
   auto setup_mock_ext2_layout(kernel::tests::devices::block_device & device) -> void;
+  auto setup_mock_ext2_layout(kernel::tests::devices::block_device & device,
+                              kernel::filesystem::ext2::superblock const & superblock) -> void;
+
 }  // namespace kernel::tests::filesystem::ext2
 
 #endif
\ No newline at end of file
diff --git a/kernel/src/filesystem/ext2/filesystem.cpp b/kernel/src/filesystem/ext2/filesystem.cpp
index 47e54fe..aaa50c7 100644
--- a/kernel/src/filesystem/ext2/filesystem.cpp
+++ b/kernel/src/filesystem/ext2/filesystem.cpp
@@ -89,7 +89,7 @@ namespace kernel::filesystem::ext2
     return nullptr;
   }
 
-  auto filesystem::read_inode(uint32_t inode_number) -> kstd::shared_ptr<inode>
+  auto filesystem::read_inode(uint32_t inode_number) const -> kstd::shared_ptr<inode>
   {
     auto const block_size = get_block_size();
     auto const inodes_per_group = m_superblock.inodes_per_group;
@@ -112,7 +112,8 @@ namespace kernel::filesystem::ext2
     return kstd::make_shared<inode>(this, new_inode_data);
   }
 
-  auto filesystem::map_inode_block_index_to_global_block_number(uint32_t inode_block_index, inode_data data) -> uint32_t
+  auto filesystem::map_inode_block_index_to_global_block_number(uint32_t inode_block_index, inode_data data) const
+      -> uint32_t
   {
     if (inode_block_index < constants::direct_block_count)
     {
@@ -170,7 +171,7 @@ namespace kernel::filesystem::ext2
     return 0;  // TODO BA-FS26 really correct??
   }
 
-  auto filesystem::read_block_number_at_index(uint32_t block_number, uint32_t index) -> uint32_t
+  auto filesystem::read_block_number_at_index(uint32_t block_number, uint32_t index) const -> uint32_t
   {
     uint32_t block_number_buffer = 0;
 
@@ -181,17 +182,22 @@ namespace kernel::filesystem::ext2
     return block_number_buffer;
   }
 
-  auto filesystem::get_block_size() -> size_t
+  auto filesystem::get_block_size() const -> size_t
   {
     return constants::base_block_size << m_superblock.log_block_size;
   }
 
-  auto filesystem::get_inode_size() -> size_t
+  auto filesystem::get_revision_level() const -> size_t
   {
-    return m_superblock.rev_level == 0 ? 128 : m_superblock.inode_size;
+    return m_superblock.rev_level;
   }
 
-  auto filesystem::get_inode_block_count(inode_data const & data) -> uint32_t
+  auto filesystem::get_inode_size() const -> size_t
+  {
+    return get_revision_level() == constants::good_old_revision ? 128 : m_superblock.inode_size;
+  }
+
+  auto filesystem::get_inode_block_count(inode_data const & data) const -> uint32_t
   {
     return data.blocks / (2 << m_superblock.log_block_size);
   }
diff --git a/kernel/src/filesystem/ext2/inode.cpp b/kernel/src/filesystem/ext2/inode.cpp
index 1914c70..303838e 100644
--- a/kernel/src/filesystem/ext2/inode.cpp
+++ b/kernel/src/filesystem/ext2/inode.cpp
@@ -13,7 +13,7 @@
 
 namespace kernel::filesystem::ext2
 {
-  inode::inode(filesystem * fs, inode_data const & data)
+  inode::inode(filesystem const * fs, inode_data const & data)
       : m_filesystem(fs)
       , m_data(data)
   {
@@ -25,11 +25,10 @@ namespace kernel::filesystem::ext2
 
   auto inode::read(void * buffer, size_t offset, size_t size) const -> size_t
   {
-    // TODO BA-FS26 use revision 1 size
-    auto const max_readable = static_cast<size_t>(m_data.size) - offset;
+    auto const max_readable = get_size() - offset;
     auto const requested_size = std::min(size, max_readable);
 
-    if (is_symbolic_link() && m_data.size <= sizeof(m_data.block))
+    if (is_symbolic_link() && get_size() <= sizeof(m_data.block))
     {
       auto inline_target = reinterpret_cast<uint8_t const *>(m_data.block.data());
       kstd::libc::memcpy(static_cast<uint8_t *>(buffer), inline_target + offset, requested_size);
@@ -91,4 +90,17 @@ namespace kernel::filesystem::ext2
   {
     return (m_data.mode & constants::mode_mask) == constants::mode_symbolic_link;
   }
+
+  auto inode::get_size() const -> size_t
+  {
+    uint64_t size = m_data.size;
+
+    if (m_filesystem->get_revision_level() > constants::good_old_revision && is_regular())
+    {
+      size |= static_cast<uint64_t>(m_data.dir_acl) << 32;
+    }
+
+    return size;
+  }
+
 }  // namespace kernel::filesystem::ext2
diff --git a/kernel/src/filesystem/ext2/inode.tests.cpp b/kernel/src/filesystem/ext2/inode.tests.cpp
index 49ba21b..783d930 100644
--- a/kernel/src/filesystem/ext2/inode.tests.cpp
+++ b/kernel/src/filesystem/ext2/inode.tests.cpp
@@ -1,5 +1,6 @@
 #include <kernel/filesystem/ext2/inode.hpp>
 
+#include "kernel/filesystem/ext2/superblock.hpp"
 #include <kernel/devices/storage/management.hpp>
 #include <kernel/filesystem/device_inode.hpp>
 #include <kernel/filesystem/ext2/filesystem.hpp>
@@ -127,7 +128,7 @@ SCENARIO_METHOD(kernel::tests::filesystem::storage_boot_module_fixture, "Ext2 in
 
 SCENARIO("Ext2 inode read stops when block mapping resolves to zero", "[filesystem][ext2][inode]")
 {
-  auto const block_size = 1024;
+  auto const block_size = 1024uz;
   GIVEN("an ext2 inode without mapped data blocks")
   {
     auto device = kstd::make_shared<kernel::tests::devices::block_device>(0, 0, "mock", block_size, 64 * block_size);
@@ -156,7 +157,7 @@ SCENARIO("Ext2 inode read stops when block mapping resolves to zero", "[filesyst
 
 SCENARIO("Ext2 inode read across block boundaries", "[filesystem][ext2][inode]")
 {
-  auto const block_size = 1024;
+  auto const block_size = 1024uz;
   GIVEN("an ext2 inode with two direct blocks and a block size of 1024 bytes")
   {
     auto device = kstd::make_shared<kernel::tests::devices::block_device>(0, 0, "mock", block_size, 64 * block_size);
@@ -203,3 +204,88 @@ SCENARIO("Ext2 inode write is not implemented", "[filesystem][ext2][inode]")
     }
   }
 }
+
+SCENARIO("Ext2 inode get_size() correctly returns size depending on revision level", "[filesystem][ext2][inode]")
+{
+  auto const block_size = 1024uz;
+
+  auto superblock = kernel::filesystem::ext2::superblock{};
+  superblock.magic = kernel::filesystem::ext2::constants::magic_number;
+  superblock.log_block_size = 0;
+  superblock.blocks_count = 64;
+  superblock.blocks_per_group = 64;
+  superblock.inodes_per_group = 32;
+  superblock.inode_size = 128;
+
+  GIVEN("an ext2 inode with good old revision and inode_data.size = 256, inode_data.dir_acl = 32")
+  {
+    superblock.rev_level = 0;
+
+    auto device = kstd::make_shared<kernel::tests::devices::block_device>(0, 0, "mock", block_size, 64 * block_size);
+    REQUIRE(device != nullptr);
+    kernel::tests::filesystem::ext2::setup_mock_ext2_layout(*device, superblock);
+
+    auto dev_inode = kstd::make_shared<kernel::filesystem::device_inode>(device);
+
+    auto fs = kernel::filesystem::ext2::filesystem{};
+    REQUIRE(fs.mount(dev_inode) == kernel::filesystem::filesystem::operation_result::success);
+
+    auto data = kernel::filesystem::ext2::inode_data{};
+    data.size = 256;
+    data.dir_acl = 32;
+
+    THEN("the inode size is 256 if mode = regular")
+    {
+      data.mode = kernel::filesystem::ext2::constants::mode_regular;
+
+      auto inode = kernel::filesystem::ext2::inode{&fs, data};
+
+      REQUIRE(inode.get_size() == 256);
+    }
+
+    THEN("the inode size is 256 if mode = directory")
+    {
+      data.mode = kernel::filesystem::ext2::constants::mode_directory;
+
+      auto inode = kernel::filesystem::ext2::inode{&fs, data};
+
+      REQUIRE(inode.get_size() == 256);
+    }
+  }
+
+  GIVEN("an ext2 inode with good dynamic revision and inode_data.size = 256, inode_data.dir_acl = 32")
+  {
+    superblock.rev_level = 1;
+
+    auto device = kstd::make_shared<kernel::tests::devices::block_device>(0, 0, "mock", block_size, 64 * block_size);
+    REQUIRE(device != nullptr);
+    kernel::tests::filesystem::ext2::setup_mock_ext2_layout(*device, superblock);
+
+    auto dev_inode = kstd::make_shared<kernel::filesystem::device_inode>(device);
+
+    auto fs = kernel::filesystem::ext2::filesystem{};
+    REQUIRE(fs.mount(dev_inode) == kernel::filesystem::filesystem::operation_result::success);
+
+    auto data = kernel::filesystem::ext2::inode_data{};
+    data.size = 256;
+    data.dir_acl = 32;
+
+    THEN("the inode size is 256 if mode = regular")
+    {
+      data.mode = kernel::filesystem::ext2::constants::mode_regular;
+
+      auto inode = kernel::filesystem::ext2::inode{&fs, data};
+
+      REQUIRE(inode.get_size() == 0x0000'0020'0000'0100);
+    }
+
+    THEN("the inode size is 256 if mode = directory")
+    {
+      data.mode = kernel::filesystem::ext2::constants::mode_directory;
+
+      auto inode = kernel::filesystem::ext2::inode{&fs, data};
+
+      REQUIRE(inode.get_size() == 256);
+    }
+  }
+}
\ No newline at end of file
diff --git a/kernel/src/test_support/filesystem/ext2.cpp b/kernel/src/test_support/filesystem/ext2.cpp
index 5a27b63..52b6efe 100644
--- a/kernel/src/test_support/filesystem/ext2.cpp
+++ b/kernel/src/test_support/filesystem/ext2.cpp
@@ -43,6 +43,12 @@ namespace kernel::tests::filesystem::ext2
     superblock.inodes_per_group = 32;
     superblock.rev_level = 1;
     superblock.inode_size = 128;
+    setup_mock_ext2_layout(device, superblock);
+  }
+
+  auto setup_mock_ext2_layout(kernel::tests::devices::block_device & device,
+                              kernel::filesystem::ext2::superblock const & superblock) -> void
+  {
     write_bytes(device, kernel::filesystem::ext2::constants::superblock_offset, &superblock, sizeof(superblock));
 
     auto group_descriptor = kernel::filesystem::ext2::block_group_descriptor{};