kernel/fs: ext2: cosmetic inode cleanup

3 files changed, 160 insertions, 123 deletions

diff --git a/kernel/kernel/filesystem/ext2/filesystem.cpp b/kernel/kernel/filesystem/ext2/filesystem.cpp
index bc935e0f..5104e8cc 100644
--- a/kernel/kernel/filesystem/ext2/filesystem.cpp
+++ b/kernel/kernel/filesystem/ext2/filesystem.cpp
@@ -429,7 +429,7 @@ namespace kernel::filesystem::ext2
     }
 
     auto size = block_count(directory, state) * block_size(state);
-    directory.data_mutable().size = size.value;
+    directory.data().size = size.value;
     return write_inode(directory.number(), directory.data(), state);
   }
 
@@ -465,7 +465,7 @@ namespace kernel::filesystem::ext2
     (*dot_dot)->rec_len = remainder.value;
 
     // '..' inside the new dir counts as an extra hard link to the parent
-    auto & parent_inode_data = parent.data_mutable();
+    auto & parent_inode_data = parent.data();
     parent_inode_data.links_count++;
 
     return write_inode(parent.number(), parent_inode_data, state)
diff --git a/kernel/kernel/filesystem/ext2/inode.cpp b/kernel/kernel/filesystem/ext2/inode.cpp
index 2d4091aa..cfc22989 100644
--- a/kernel/kernel/filesystem/ext2/inode.cpp
+++ b/kernel/kernel/filesystem/ext2/inode.cpp
@@ -47,20 +47,19 @@ namespace kernel::filesystem::ext2
       return kstd::failure(filesystem.error());
     }
 
-    auto driver_data = get_driver_data();
-    if (!driver_data)
+    auto state = get_driver_data();
+    if (!state)
     {
-      return kstd::failure(driver_data.error());
+      return kstd::failure(state.error());
     }
-    auto state = static_pointer_cast<mount_state const>(*driver_data);
 
-    auto backing_inode = state->backing_inode;
-    if (!state->backing_inode)
+    auto backing_inode = (*state)->backing_inode;
+    if (!(*state)->backing_inode)
     {
       return kstd::failure(vfs_errc::not_mounted);
     }
 
-    auto block_size = (*filesystem)->block_size(*state);
+    auto block_size = (*filesystem)->block_size(**state);
     auto block_index = offset / block_size;
     auto in_block_offset = offset % block_size;
 
@@ -69,7 +68,7 @@ namespace kernel::filesystem::ext2
     while (bytes_read < requested_size)
     {
       auto const block_number =
-          (*filesystem)->map_inode_block_index_to_global_block_number(block_index, m_data, *state);
+          (*filesystem)->map_inode_block_index_to_global_block_number(block_index, m_data, **state);
       if (!block_number)
       {
         break;
@@ -121,22 +120,21 @@ namespace kernel::filesystem::ext2
       return kstd::failure(filesystem.error());
     }
 
-    auto driver_data = get_driver_data();
-    if (!driver_data)
+    auto state = get_driver_data();
+    if (!state)
     {
-      return kstd::failure(driver_data.error());
+      return kstd::failure(state.error());
     }
-    auto state = static_pointer_cast<mount_state>(*driver_data);
 
-    auto backing_inode = state->backing_inode;
-    if (!state->backing_inode)
+    auto backing_inode = (*state)->backing_inode;
+    if (!(*state)->backing_inode)
     {
       return kstd::failure(vfs_errc::not_mounted);
     }
 
-    auto block_size = (*filesystem)->block_size(*state);
+    auto block_size = (*filesystem)->block_size(**state);
 
-    auto const current_block_count = (*filesystem)->block_count(*this, *state);
+    auto const current_block_count = (*filesystem)->block_count(*this, **state);
     auto const max_new_inode_size_without_new_blocks = current_block_count * block_size;
 
     if (new_inode_size > max_new_inode_size_without_new_blocks)
@@ -159,7 +157,7 @@ namespace kernel::filesystem::ext2
     while (bytes_written < kstd::bytes{buffer.size()})
     {
       auto const block_number =
-          (*filesystem)->map_inode_block_index_to_global_block_number(block_index, m_data, *state);
+          (*filesystem)->map_inode_block_index_to_global_block_number(block_index, m_data, **state);
       // TODO BA-FS26 if blocknumber == 0 --> handle sparse file
       if (!block_number)
       {
@@ -185,7 +183,7 @@ namespace kernel::filesystem::ext2
     }
 
     set_size(std::max(this->size(), offset + bytes_written));
-    if (auto write_result = (*filesystem)->write_inode(m_inode_number, m_data, *state); !write_result)
+    if (auto write_result = (*filesystem)->write_inode(m_inode_number, m_data, **state); !write_result)
     {
       return kstd::failure(write_result.error());
     }
@@ -193,54 +191,19 @@ namespace kernel::filesystem::ext2
     return bytes_written;
   }
 
-  auto inode::append_blocks(size_t count) -> bool
+  auto inode::is_block_device() const -> bool
   {
-    auto filesystem = get_filesystem();
-    if (!filesystem)
-    {
-      return false;
-    }
-
-    auto driver_data = get_driver_data();
-    if (!driver_data)
-    {
-      return false;
-    }
-
-    auto state = static_pointer_cast<mount_state>(*driver_data);
-
-    auto new_blocks = (*filesystem)->allocate_blocks(count, *state);
-    if (!new_blocks)
-    {
-      return false;
-    }
-
-    for (auto i = 0uz; i < new_blocks->size(); ++i)
-    {
-      auto const block_index = (*filesystem)->block_count(*this, *state) + i;
-      auto const global_block_number = new_blocks->at(i);
-
-      if (auto write_result =
-              (*filesystem)
-                  ->write_global_block_number_to_inode_block_index(block_index, m_data, global_block_number, *state);
-          !write_result)
-      {
-        return false;
-      }
-    }
-
-    (*filesystem)->update_inode_block_count(m_data, count, *state);
-    return true;
+    return (m_data.mode & constants::mode_mask) == constants::mode_block_device;
   }
 
-  [[nodiscard]] auto inode::data() const -> inode_data const &
+  auto inode::is_character_device() const -> bool
   {
-    return m_data;
+    return (m_data.mode & constants::mode_mask) == constants::mode_character_device;
   }
 
-  [[nodiscard]] auto inode::data_mutable() -> inode_data &
+  auto inode::is_directory() const -> bool
   {
-    return m_data;
+    return (m_data.mode & constants::mode_mask) == constants::mode_directory;
   }
 
   auto inode::is_regular() const -> bool
@@ -248,24 +211,24 @@ namespace kernel::filesystem::ext2
     return (m_data.mode & constants::mode_mask) == constants::mode_regular;
   }
 
-  auto inode::is_directory() const -> bool
-  {
-    return (m_data.mode & constants::mode_mask) == constants::mode_directory;
-  }
-
   auto inode::is_symbolic_link() const -> bool
   {
     return (m_data.mode & constants::mode_mask) == constants::mode_symbolic_link;
   }
 
-  auto inode::is_block_device() const -> bool
+  auto inode::raw_device() const -> std::optional<kapi::filesystem::device_number>
   {
-    return (m_data.mode & constants::mode_mask) == constants::mode_block_device;
-  }
+    if (!is_device())
+    {
+      return std::nullopt;
+    }
 
-  auto inode::is_character_device() const -> bool
-  {
-    return (m_data.mode & constants::mode_mask) == constants::mode_character_device;
+    auto const raw_device_number = m_data.block[0];
+
+    return kapi::filesystem::device_number{
+      .major = static_cast<std::uint8_t>((raw_device_number >> 8) & 0xff),
+      .minor = static_cast<std::uint8_t>(raw_device_number & 0xff),
+    };
   }
 
   auto inode::status() const -> kstd::result<kapi::filesystem::file_status>
@@ -284,45 +247,42 @@ namespace kernel::filesystem::ext2
     return result;
   }
 
-  auto inode::raw_device() const -> std::optional<kapi::filesystem::device_number>
-  {
-    if (!is_device())
-    {
-      return std::nullopt;
-    }
-
-    auto const raw_device_number = m_data.block[0];
-
-    return kapi::filesystem::device_number{
-      .major = static_cast<std::uint8_t>((raw_device_number >> 8) & 0xff),
-      .minor = static_cast<std::uint8_t>(raw_device_number & 0xff),
-    };
-  }
-
-  auto inode::size() const -> kstd::bytes
+  auto inode::append_blocks(size_t count) -> bool
   {
-    uint64_t size = m_data.size;
-
     auto filesystem = get_filesystem();
     if (!filesystem)
     {
-      return 0_B;
+      return false;
     }
 
-    auto driver_data = get_driver_data();
-    if (!driver_data)
+    auto state = get_driver_data();
+    if (!state)
     {
-      return 0_B;
+      return false;
     }
 
-    auto state = static_pointer_cast<mount_state const>(*driver_data);
+    auto new_blocks = (*filesystem)->allocate_blocks(count, **state);
+    if (!new_blocks)
+    {
+      return false;
+    }
 
-    if ((*filesystem)->revision_level(*state) > constants::good_old_revision && is_regular())
+    for (auto i = 0uz; i < new_blocks->size(); ++i)
     {
-      size |= static_cast<uint64_t>(m_data.dir_acl) << 32;
+      auto const block_index = (*filesystem)->block_count(*this, **state) + i;
+      auto const global_block_number = new_blocks->at(i);
+
+      if (auto write_result =
+              (*filesystem)
+                  ->write_global_block_number_to_inode_block_index(block_index, m_data, global_block_number, **state);
+          !write_result)
+      {
+        return false;
+      }
     }
 
-    return kstd::bytes{size};
+    (*filesystem)->update_inode_block_count(m_data, count, **state);
+    return true;
   }
 
   auto inode::set_size(kstd::bytes new_size) -> void
@@ -348,6 +308,40 @@ namespace kernel::filesystem::ext2
     m_data.size = static_cast<uint32_t>(new_size.value);
   }
 
+  [[nodiscard]] auto inode::data() -> inode_data &
+  {
+    return m_data;
+  }
+
+  [[nodiscard]] auto inode::data() const -> inode_data const &
+  {
+    return m_data;
+  }
+
+  auto inode::size() const -> kstd::bytes
+  {
+    uint64_t size = m_data.size;
+
+    auto filesystem = get_filesystem();
+    if (!filesystem)
+    {
+      return 0_B;
+    }
+
+    auto state = get_driver_data();
+    if (!state)
+    {
+      return 0_B;
+    }
+
+    if ((*filesystem)->revision_level(**state) > constants::good_old_revision && is_regular())
+    {
+      size |= static_cast<uint64_t>(m_data.dir_acl) << 32;
+    }
+
+    return kstd::bytes{size};
+  }
+
   [[nodiscard]] auto inode::number() const -> uint32_t
   {
     return m_inode_number;
@@ -373,21 +367,21 @@ namespace kernel::filesystem::ext2
     return kstd::failure(vfs_errc::not_mounted);
   }
 
-  auto inode::get_driver_data() -> kstd::result<kstd::shared_ptr<void>>
+  auto inode::get_driver_data() -> kstd::result<kstd::shared_ptr<mount_state>>
   {
     if (auto mount = owning_mount().lock())
     {
-      return mount->driver_data();
+      return static_pointer_cast<mount_state>(mount->driver_data());
     }
 
     return kstd::failure(vfs_errc::not_mounted);
   }
 
-  auto inode::get_driver_data() const -> kstd::result<kstd::shared_ptr<void const>>
+  auto inode::get_driver_data() const -> kstd::result<kstd::shared_ptr<mount_state const>>
   {
     if (auto mount = owning_mount().lock())
     {
-      return mount->driver_data();
+      return static_pointer_cast<mount_state const>(mount->driver_data());
     }
 
     return kstd::failure(vfs_errc::not_mounted);
diff --git a/kernel/kernel/filesystem/ext2/inode.hpp b/kernel/kernel/filesystem/ext2/inode.hpp
index 7f668833..735905e9 100644
--- a/kernel/kernel/filesystem/ext2/inode.hpp
+++ b/kernel/kernel/filesystem/ext2/inode.hpp
@@ -1,6 +1,7 @@
 #ifndef TEACHOS_KERNEL_FILESYSTEM_EXT2_INODE_HPP
 #define TEACHOS_KERNEL_FILESYSTEM_EXT2_INODE_HPP
 
+#include <kernel/filesystem/ext2/mount_state.hpp>
 #include <kernel/filesystem/inode.hpp>
 
 #include <kapi/filesystem.hpp>
@@ -44,32 +45,33 @@ namespace kernel::filesystem::ext2
 
   struct inode : kernel::filesystem::inode
   {
+    //! @name Construction/Destruction
+    //! @{
+
     //! Create an ext2 inode associated with the given filesystem.
     //!
     //! @param inode_number The inode number on disk.
     //! @param data The data associated with this inode, read from the disk.
     explicit inode(uint32_t inode_number, inode_data const & data);
 
+    //! @}
+
+    //! @name I/O Operations
+    //! @{
+
     [[nodiscard]] auto read(std::span<std::byte> buffer, kstd::bytes offset) const
         -> kstd::result<kstd::bytes> override;
 
     auto write(std::span<std::byte const> buffer, kstd::bytes offset) -> kstd::result<kstd::bytes> override;
 
-    //! Append the specified number of blocks to this inode.
-    //!
-    //! @param count The number of blocks to append.
-    //! @return true if the blocks were successfully appended, false otherwise.
-    auto append_blocks(size_t count) -> bool;
+    //! @}
 
-    //! Get the data associated with this inode.
-    //!
-    //! @return A const reference to the inode data.
-    [[nodiscard]] auto data() const -> inode_data const &;
+    //! @name Property Checking
+    //! @{
 
-    //! Get the data associated with this inode.
-    //!
-    //! @return A reference to the inode data.
-    [[nodiscard]] auto data_mutable() -> inode_data &;
+    [[nodiscard]] auto is_block_device() const -> bool override;
+
+    [[nodiscard]] auto is_character_device() const -> bool override;
 
     [[nodiscard]] auto is_directory() const -> bool override;
 
@@ -77,37 +79,78 @@ namespace kernel::filesystem::ext2
 
     [[nodiscard]] auto is_symbolic_link() const -> bool override;
 
-    [[nodiscard]] auto is_block_device() const -> bool override;
+    //! @}
 
-    [[nodiscard]] auto is_character_device() const -> bool override;
+    //! @name POSIX Information Access
+    //! @{
+
+    [[nodiscard]] auto raw_device() const -> std::optional<kapi::filesystem::device_number> override;
 
     [[nodiscard]] auto status() const -> kstd::result<kapi::filesystem::file_status> override;
 
-    [[nodiscard]] auto raw_device() const -> std::optional<kapi::filesystem::device_number> override;
+    //! @}
 
-    //! Get the size of the file represented by this inode.
+    //! @name Property Manipulation
+    //! @{
+
+    //! Append the specified number of blocks to this inode.
     //!
-    //! @return The size of the file in bytes.
-    [[nodiscard]] auto size() const -> kstd::bytes;
+    //! @param count The number of blocks to append.
+    //! @return true if the blocks were successfully appended, false otherwise.
+    auto append_blocks(size_t count) -> bool;
 
     //! Set the size of the file represented by this inode.
     //!
     //! @param new_size The new size of the file in bytes.
     auto set_size(kstd::bytes new_size) -> void;
 
+    //! @}
+
+    //! @name Ext2 Information Queries
+    //! @{
+
+    //! Get the data associated with this inode.
+    //!
+    //! @return A reference to the inode data.
+    [[nodiscard]] auto data() -> inode_data &;
+
+    //! Get the data associated with this inode.
+    //!
+    //! @return A const reference to the inode data.
+    [[nodiscard]] auto data() const -> inode_data const &;
+
+    //! Get the size of the file represented by this inode.
+    //!
+    //! @return The size of the file in bytes.
+    [[nodiscard]] auto size() const -> kstd::bytes;
+
     //! @brief Get the inode number of this inode.
     //!
     //! @return The inode number.
     [[nodiscard]] auto number() const -> uint32_t;
 
+    //! @}
+
   private:
+    //! Get the filesystem driver associated with this inode.
+    //!
+    //! @return The filesystem driver on success, and error otherwise.
     [[nodiscard]] auto get_filesystem() -> kstd::result<kstd::shared_ptr<filesystem>>;
 
+    //! Get the filesystem driver associated with this inode.
+    //!
+    //! @return The filesystem driver on success, and error otherwise.
     [[nodiscard]] auto get_filesystem() const -> kstd::result<kstd::shared_ptr<filesystem const>>;
 
-    [[nodiscard]] auto get_driver_data() -> kstd::result<kstd::shared_ptr<void>>;
+    //! Get the filesystem driver data associated with this inode.
+    //!
+    //! @return The filesystem driver data on success, and error otherwise.
+    [[nodiscard]] auto get_driver_data() -> kstd::result<kstd::shared_ptr<mount_state>>;
 
-    [[nodiscard]] auto get_driver_data() const -> kstd::result<kstd::shared_ptr<void const>>;
+    //! Get the filesystem driver data associated with this inode.
+    //!
+    //! @return The filesystem driver data on success, and error otherwise.
+    [[nodiscard]] auto get_driver_data() const -> kstd::result<kstd::shared_ptr<mount_state const>>;
 
     //! The inode number on disk.
     uint32_t m_inode_number{};