From b3209ac2564f21f3b78ecf5e0c05ca346a4a4276 Mon Sep 17 00:00:00 2001 From: Lukas Oesch Date: Tue, 28 Apr 2026 10:49:34 +0200 Subject: refactor inode kind --- kernel/include/kernel/filesystem/devfs/inode.hpp | 10 +++--- kernel/include/kernel/filesystem/device_inode.hpp | 5 +++ kernel/include/kernel/filesystem/ext2/inode.hpp | 21 +++++++++--- kernel/include/kernel/filesystem/inode.hpp | 25 +++------------ .../include/kernel/filesystem/open_file_table.hpp | 4 +-- kernel/include/kernel/filesystem/rootfs/inode.hpp | 10 +++--- .../kernel/test_support/filesystem/inode.hpp | 4 +-- kernel/src/filesystem/devfs/inode.cpp | 11 +++---- kernel/src/filesystem/device_inode.cpp | 9 ++++-- kernel/src/filesystem/ext2/filesystem.cpp | 36 +++------------------ kernel/src/filesystem/ext2/inode.cpp | 21 ++++++++++-- kernel/src/filesystem/ext2/inode.tests.cpp | 37 +++++++++++++++------- kernel/src/filesystem/inode.cpp | 10 ++---- kernel/src/filesystem/rootfs/inode.cpp | 9 +++--- kernel/src/test_support/filesystem/inode.cpp | 9 +++--- 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 @@ -13,11 +13,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. @@ -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 const &; + /** + // TODO BA-FS26 + */ + [[nodiscard]] auto is_device() const -> bool override; + private: kstd::shared_ptr 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 const & f) -> int; + auto add_file(kstd::shared_ptr 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 @@ -21,11 +21,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. @@ -57,6 +52,11 @@ namespace kernel::filesystem::rootfs */ auto lookup_child(std::string_view name) -> kstd::shared_ptr; + /** + // TODO BA-FS26 + */ + [[nodiscard]] auto is_directory() const -> bool override; + private: kstd::vector>> 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 -#include - #include 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 #include -#include #include #include @@ -13,8 +12,7 @@ namespace kernel::filesystem { device_inode::device_inode(kstd::shared_ptr 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 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 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(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(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(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(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(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 -- cgit v1.2.3