refactor inode kind

15 files changed, 113 insertions, 108 deletions

diff --git a/kernel/include/kernel/filesystem/devfs/inode.hpp b/kernel/include/kernel/filesystem/devfs/inode.hpp
index 0fab280..5589730 100644
--- a/kernel/include/kernel/filesystem/devfs/inode.hpp
+++ b/kernel/include/kernel/filesystem/devfs/inode.hpp
@@ -14,11 +14,6 @@ namespace kernel::filesystem::devfs
   struct inode : kernel::filesystem::inode
   {
     /**
-    @brief Create a devfs inode. The inode is initialized with the appropriate kind (directory).
-    */
-    inode();
-
-    /**
     @brief Reads from the devfs directory inode.
     @param buffer Destination buffer.
     @param offset Read offset in bytes.
@@ -35,6 +30,11 @@ namespace kernel::filesystem::devfs
     @return Number of bytes written (always 0 because writes are not supported for this inode).
     */
     auto write(void const * buffer, size_t offset, size_t size) -> size_t override;
+
+    /**
+    // % TODO BA-FS26
+    */
+    [[nodiscard]] auto is_directory() const -> bool override;
   };
 }  // namespace kernel::filesystem::devfs
 
diff --git a/kernel/include/kernel/filesystem/device_inode.hpp b/kernel/include/kernel/filesystem/device_inode.hpp
index 2f79fca..fb60524 100644
--- a/kernel/include/kernel/filesystem/device_inode.hpp
+++ b/kernel/include/kernel/filesystem/device_inode.hpp
@@ -50,6 +50,11 @@ namespace kernel::filesystem
     */
     [[nodiscard]] auto device() const -> kstd::shared_ptr<kapi::devices::device> const &;
 
+    /**
+    // TODO BA-FS26
+    */
+    [[nodiscard]] auto is_device() const -> bool override;
+
   private:
     kstd::shared_ptr<kapi::devices::device> m_device;
   };
diff --git a/kernel/include/kernel/filesystem/ext2/inode.hpp b/kernel/include/kernel/filesystem/ext2/inode.hpp
index 9291eea..64cdb06 100644
--- a/kernel/include/kernel/filesystem/ext2/inode.hpp
+++ b/kernel/include/kernel/filesystem/ext2/inode.hpp
@@ -42,8 +42,10 @@ namespace kernel::filesystem::ext2
   {
     /**
     @brief Create an ext2 inode associated with the given filesystem.
+    @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);
+    explicit inode(filesystem * 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.
@@ -65,12 +67,23 @@ namespace kernel::filesystem::ext2
     auto write(void const * buffer, size_t offset, size_t size) -> size_t override;
 
     /**
-    @brief The raw inode data as read from the disk.
-    */
-    inode_data m_data{};
+    // TODO BA-FS26
+     */
+    [[nodiscard]] auto data() const -> inode_data const &;
+
+    /**
+    // TODO BA-FS26
+     */
+    [[nodiscard]] auto is_directory() const -> bool override;
+
+    /**
+    // TODO BA-FS26
+     */
+    [[nodiscard]] auto is_regular() const -> bool override;
 
   private:
     filesystem * m_filesystem;
+    inode_data m_data{};
   };
 }  // namespace kernel::filesystem::ext2
 
diff --git a/kernel/include/kernel/filesystem/inode.hpp b/kernel/include/kernel/filesystem/inode.hpp
index 5208be2..d071a6b 100644
--- a/kernel/include/kernel/filesystem/inode.hpp
+++ b/kernel/include/kernel/filesystem/inode.hpp
@@ -11,20 +11,9 @@ namespace kernel::filesystem
   struct inode
   {
     /**
-    @brief Represents the kind of an inode.
+    @brief Create an inode.
     */
-    enum class inode_kind
-    {
-      regular,
-      directory,
-      device
-    };
-
-    /**
-    @brief Create an inode with the given @p kind.
-    @param kind The kind of the inode.
-    */
-    explicit inode(inode_kind kind);
+    inode() = default;
 
     /**
     @brief Virtual destructor for the inode.
@@ -55,23 +44,19 @@ namespace kernel::filesystem
     @brief Returns whether the inode is a directory.
     @return true if the inode is a directory, false otherwise.
     */
-    [[nodiscard]] auto is_directory() const -> bool;
+    [[nodiscard]] virtual auto is_directory() const -> bool;
 
     /**
     @brief Returns whether the inode is a regular file.
     @return true if the inode is a regular file, false otherwise.
     */
-    [[nodiscard]] auto is_regular() const -> bool;
+    [[nodiscard]] virtual auto is_regular() const -> bool;
 
     /**
     @brief Returns whether the inode is a device.
     @return true if the inode is a device, false otherwise.
     */
-    [[nodiscard]] auto is_device() const -> bool;
-
-    // TODO BA-FS26 avoid public member
-  public:
-    inode_kind m_kind{inode_kind::regular};
+    [[nodiscard]] virtual auto is_device() const -> bool;
   };
 }  // namespace kernel::filesystem
 
diff --git a/kernel/include/kernel/filesystem/open_file_table.hpp b/kernel/include/kernel/filesystem/open_file_table.hpp
index 2f9a421..694f3b6 100644
--- a/kernel/include/kernel/filesystem/open_file_table.hpp
+++ b/kernel/include/kernel/filesystem/open_file_table.hpp
@@ -34,10 +34,10 @@ namespace kernel::filesystem
 
     /**
     @brief Add a file to the open file table.
-    @param f The file descriptor to add.
+    @param fd The file descriptor to add.
     @return The file descriptor index assigned to the file, or -1 on failure.
     */
-    auto add_file(kstd::shared_ptr<open_file_descriptor> const & f) -> int;
+    auto add_file(kstd::shared_ptr<open_file_descriptor> const & fd) -> int;
 
     /**
     @brief Get a file from the open file table.
diff --git a/kernel/include/kernel/filesystem/rootfs/inode.hpp b/kernel/include/kernel/filesystem/rootfs/inode.hpp
index 37d0a30..e7c7eff 100644
--- a/kernel/include/kernel/filesystem/rootfs/inode.hpp
+++ b/kernel/include/kernel/filesystem/rootfs/inode.hpp
@@ -22,11 +22,6 @@ namespace kernel::filesystem::rootfs
   struct inode : kernel::filesystem::inode
   {
     /**
-    @brief Create a rootfs inode. The inode is initialized with the appropriate kind (directory).
-    */
-    inode();
-
-    /**
     @brief Reads from the rootfs directory inode.
     @param buffer Destination buffer.
     @param offset Read offset in bytes.
@@ -57,6 +52,11 @@ namespace kernel::filesystem::rootfs
     */
     auto lookup_child(std::string_view name) -> kstd::shared_ptr<inode>;
 
+    /**
+    // TODO BA-FS26
+    */
+    [[nodiscard]] auto is_directory() const -> bool override;
+
   private:
     kstd::vector<std::pair<kstd::string, kstd::shared_ptr<inode>>> m_children;
   };
diff --git a/kernel/include/kernel/test_support/filesystem/inode.hpp b/kernel/include/kernel/test_support/filesystem/inode.hpp
index 9d17917..8a76437 100644
--- a/kernel/include/kernel/test_support/filesystem/inode.hpp
+++ b/kernel/include/kernel/test_support/filesystem/inode.hpp
@@ -9,10 +9,10 @@ namespace kernel::tests::filesystem
 {
   struct inode : kernel::filesystem::inode
   {
-    inode();
-
     auto read(void * buffer, size_t offset, size_t size) const -> size_t override;
     auto write(void const * buffer, size_t offset, size_t size) -> size_t override;
+
+    [[nodiscard]] auto is_regular() const -> bool override;
   };
 }  // namespace kernel::tests::filesystem
 
diff --git a/kernel/src/filesystem/devfs/inode.cpp b/kernel/src/filesystem/devfs/inode.cpp
index 0ed66ad..2029a7f 100644
--- a/kernel/src/filesystem/devfs/inode.cpp
+++ b/kernel/src/filesystem/devfs/inode.cpp
@@ -1,15 +1,9 @@
 #include <kernel/filesystem/devfs/inode.hpp>
 
-#include <kernel/filesystem/inode.hpp>
-
 #include <cstddef>
 
 namespace kernel::filesystem::devfs
 {
-  inode::inode()
-      : kernel::filesystem::inode(inode_kind::directory)
-  {}
-
   auto inode::read(void * /*buffer*/, size_t /*offset*/, size_t /*size*/) const -> size_t
   {
     return 0;
@@ -19,4 +13,9 @@ namespace kernel::filesystem::devfs
   {
     return 0;
   }
+
+  auto inode::is_directory() const -> bool
+  {
+    return true;
+  }
 }  // namespace kernel::filesystem::devfs
\ No newline at end of file
diff --git a/kernel/src/filesystem/device_inode.cpp b/kernel/src/filesystem/device_inode.cpp
index 3bafe06..81a784c 100644
--- a/kernel/src/filesystem/device_inode.cpp
+++ b/kernel/src/filesystem/device_inode.cpp
@@ -1,7 +1,6 @@
 #include <kernel/filesystem/device_inode.hpp>
 
 #include <kernel/devices/block_device_utils.hpp>
-#include <kernel/filesystem/inode.hpp>
 
 #include <kapi/devices/device.hpp>
 #include <kapi/system.hpp>
@@ -13,8 +12,7 @@
 namespace kernel::filesystem
 {
   device_inode::device_inode(kstd::shared_ptr<kapi::devices::device> const & device)
-      : inode(inode_kind::device)
-      , m_device(device)
+      : m_device(device)
   {
     if (!device)
     {
@@ -51,4 +49,9 @@ namespace kernel::filesystem
     return m_device;
   }
 
+  auto device_inode::is_device() const -> bool
+  {
+    return true;
+  }
+
 }  // namespace kernel::filesystem
\ No newline at end of file
diff --git a/kernel/src/filesystem/ext2/filesystem.cpp b/kernel/src/filesystem/ext2/filesystem.cpp
index 41572ee..47e54fe 100644
--- a/kernel/src/filesystem/ext2/filesystem.cpp
+++ b/kernel/src/filesystem/ext2/filesystem.cpp
@@ -16,19 +16,6 @@
 
 namespace kernel::filesystem::ext2
 {
-  namespace
-  {
-    auto S_ISREG(uint16_t mode) -> bool
-    {
-      return (mode & constants::mode_mask) == constants::mode_regular;
-    }
-
-    auto S_ISDIR(uint16_t mode) -> bool
-    {
-      return (mode & constants::mode_mask) == constants::mode_directory;
-    }
-  }  // namespace
-
   auto filesystem::mount(kstd::shared_ptr<kernel::filesystem::inode> const & backing_inode) -> operation_result
   {
     kernel::filesystem::filesystem::mount(backing_inode);
@@ -74,7 +61,7 @@ namespace kernel::filesystem::ext2
     }
 
     auto const block_size = get_block_size();
-    auto const & inode_data = ext2_parent->m_data;
+    auto const & inode_data = ext2_parent->data();
     kstd::vector<uint8_t> buffer(block_size);
 
     for (uint32_t i = 0; i < get_inode_block_count(inode_data); ++i)
@@ -119,25 +106,10 @@ namespace kernel::filesystem::ext2
     auto const inode_table_offset = static_cast<size_t>(inode_table_start_block) * block_size;
     auto const inode_offset = inode_table_offset + inode_index_within_group * get_inode_size();
 
-    auto new_inode = kstd::make_shared<inode>(this);
-    m_backing_inode->read(&new_inode->m_data, inode_offset, sizeof(inode_data));
-
-    // TODO BA-FS26 improve inode_kind really needed? or just map it to the mode bits?
-    if (S_ISREG(new_inode->m_data.mode))
-    {
-      new_inode->m_kind = inode::inode_kind::regular;
-    }
-    else if (S_ISDIR(new_inode->m_data.mode))
-    {
-      new_inode->m_kind = inode::inode_kind::directory;
-    }
-    else
-    {
-      // TODO BA-FS26 really correct??
-      return nullptr;
-    }
+    auto new_inode_data = inode_data{};
+    m_backing_inode->read(&new_inode_data, inode_offset, sizeof(inode_data));
 
-    return new_inode;
+    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
diff --git a/kernel/src/filesystem/ext2/inode.cpp b/kernel/src/filesystem/ext2/inode.cpp
index c45c41e..279c84f 100644
--- a/kernel/src/filesystem/ext2/inode.cpp
+++ b/kernel/src/filesystem/ext2/inode.cpp
@@ -11,9 +11,9 @@
 
 namespace kernel::filesystem::ext2
 {
-  inode::inode(filesystem * fs)
-      : kernel::filesystem::inode(inode_kind::regular)
-      , m_filesystem(fs)
+  inode::inode(filesystem * fs, inode_data const & data)
+      : m_filesystem(fs)
+      , m_data(data)
   {
     if (!m_filesystem)
     {
@@ -57,4 +57,19 @@ namespace kernel::filesystem::ext2
     kapi::system::panic("[EXT2] inode::write is not implemented yet");
     return 0;
   }
+
+  [[nodiscard]] auto inode::data() const -> inode_data const &
+  {
+    return m_data;
+  }
+
+  auto inode::is_regular() const -> bool
+  {
+    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;
+  }
 }  // namespace kernel::filesystem::ext2
diff --git a/kernel/src/filesystem/ext2/inode.tests.cpp b/kernel/src/filesystem/ext2/inode.tests.cpp
index 4d61790..e68352f 100644
--- a/kernel/src/filesystem/ext2/inode.tests.cpp
+++ b/kernel/src/filesystem/ext2/inode.tests.cpp
@@ -23,21 +23,34 @@ SCENARIO("Ext2 inode initialization and properties", "[filesystem][ext2][inode]"
   GIVEN("an ext2 filesystem")
   {
     auto fs = kernel::filesystem::ext2::filesystem{};
+    auto data = kernel::filesystem::ext2::inode_data{};
 
-    THEN("the inode is initialized and has the kind regular")
+    THEN("the inode is initialized with regular file mode in data and has the kind regular")
     {
-      auto inode = kernel::filesystem::ext2::inode{&fs};
+      data.mode = kernel::filesystem::ext2::constants::mode_regular;
+      auto inode = kernel::filesystem::ext2::inode(&fs, data);
+
       REQUIRE(inode.is_regular());
       REQUIRE(!inode.is_directory());
       REQUIRE(!inode.is_device());
     }
+
+    THEN("the inode is initialized with directory mode in data and has the kind directory")
+    {
+      data.mode = kernel::filesystem::ext2::constants::mode_directory;
+      auto inode = kernel::filesystem::ext2::inode(&fs, data);
+
+      REQUIRE(!inode.is_regular());
+      REQUIRE(inode.is_directory());
+      REQUIRE(!inode.is_device());
+    }
   }
 
   GIVEN("no filesystem (null pointer)")
   {
     THEN("constructing an inode with a null filesystem pointer panics")
     {
-      REQUIRE_THROWS_AS(kernel::filesystem::ext2::inode{nullptr}, kernel::tests::cpu::halt);
+      REQUIRE_THROWS_AS(kernel::filesystem::ext2::inode(nullptr, {}), kernel::tests::cpu::halt);
     }
   }
 }
@@ -102,9 +115,10 @@ SCENARIO("Ext2 inode read stops when block mapping resolves to zero", "[filesyst
     auto fs = kernel::filesystem::ext2::filesystem{};
     REQUIRE(fs.mount(dev_inode) == kernel::filesystem::filesystem::operation_result::success);
 
-    auto inode = kernel::filesystem::ext2::inode{&fs};
-    inode.m_data.blocks = 2;
-    inode.m_data.block[0] = 0;
+    auto data = kernel::filesystem::ext2::inode_data{};
+    data.blocks = 2;
+    data.block[0] = 0;
+    auto inode = kernel::filesystem::ext2::inode{&fs, data};
 
     auto buffer = kstd::vector<std::byte>(32, std::byte{0xAB});
 
@@ -130,12 +144,13 @@ SCENARIO("Ext2 inode read across block boundaries", "[filesystem][ext2][inode]")
     auto fs = kernel::filesystem::ext2::filesystem{};
     REQUIRE(fs.mount(dev_inode) == kernel::filesystem::filesystem::operation_result::success);
 
-    auto inode = kernel::filesystem::ext2::inode{&fs};
-    inode.m_data.blocks = 2;
-    inode.m_data.block[0] = 20;
+    auto inode_data = kernel::filesystem::ext2::inode_data{};
+    inode_data.blocks = 2;
+    inode_data.block[0] = 20;
     kernel::tests::filesystem::ext2::write_bytes(*device, 21 * block_size - 6, "Hello ", 6);
-    inode.m_data.block[1] = 21;
+    inode_data.block[1] = 21;
     kernel::tests::filesystem::ext2::write_bytes(*device, 21 * block_size, "World!", 6);
+    auto inode = kernel::filesystem::ext2::inode{&fs, inode_data};
 
     auto buffer = kstd::vector<std::byte>(12, std::byte{0x00});
 
@@ -155,7 +170,7 @@ SCENARIO("Ext2 inode write is not implemented", "[filesystem][ext2][inode]")
   GIVEN("an ext2 inode")
   {
     auto fs = kernel::filesystem::ext2::filesystem{};
-    auto inode = kernel::filesystem::ext2::inode{&fs};
+    auto inode = kernel::filesystem::ext2::inode{&fs, kernel::filesystem::ext2::inode_data{}};
 
     THEN("writing to the inode panics")
     {
diff --git a/kernel/src/filesystem/inode.cpp b/kernel/src/filesystem/inode.cpp
index 2f0764c..f2a7741 100644
--- a/kernel/src/filesystem/inode.cpp
+++ b/kernel/src/filesystem/inode.cpp
@@ -2,22 +2,18 @@
 
 namespace kernel::filesystem
 {
-  inode::inode(inode_kind kind)
-      : m_kind(kind)
-  {}
-
   auto inode::is_directory() const -> bool
   {
-    return m_kind == inode_kind::directory;
+    return false;
   }
 
   auto inode::is_regular() const -> bool
   {
-    return m_kind == inode_kind::regular;
+    return false;
   }
 
   auto inode::is_device() const -> bool
   {
-    return m_kind == inode_kind::device;
+    return false;
   }
 }  // namespace kernel::filesystem
\ No newline at end of file
diff --git a/kernel/src/filesystem/rootfs/inode.cpp b/kernel/src/filesystem/rootfs/inode.cpp
index eeea3fe..d099676 100644
--- a/kernel/src/filesystem/rootfs/inode.cpp
+++ b/kernel/src/filesystem/rootfs/inode.cpp
@@ -12,10 +12,6 @@
 
 namespace kernel::filesystem::rootfs
 {
-  inode::inode()
-      : kernel::filesystem::inode(inode_kind::directory)
-  {}
-
   auto inode::read(void * /*buffer*/, size_t /*offset*/, size_t /*size*/) const -> size_t
   {
     return 0;
@@ -36,4 +32,9 @@ namespace kernel::filesystem::rootfs
     auto it = std::ranges::find_if(m_children, [&](auto const & pair) { return pair.first == name; });
     return (it != m_children.end()) ? it->second : nullptr;
   }
+
+  auto inode::is_directory() const -> bool
+  {
+    return true;
+  }
 }  // namespace kernel::filesystem::rootfs
diff --git a/kernel/src/test_support/filesystem/inode.cpp b/kernel/src/test_support/filesystem/inode.cpp
index 54bd7e0..0c8d956 100644
--- a/kernel/src/test_support/filesystem/inode.cpp
+++ b/kernel/src/test_support/filesystem/inode.cpp
@@ -6,10 +6,6 @@
 
 namespace kernel::tests::filesystem
 {
-  inode::inode()
-      : kernel::filesystem::inode(inode_kind::regular)
-  {}
-
   auto inode::read(void *, size_t, size_t size) const -> size_t
   {
     return size;
@@ -19,4 +15,9 @@ namespace kernel::tests::filesystem
   {
     return size;
   }
+
+  auto inode::is_regular() const -> bool
+  {
+    return true;
+  }
 }  // namespace kernel::tests::filesystem
\ No newline at end of file