From 2833fa2a2d2bf1f98f627503e52531615d1c1496 Mon Sep 17 00:00:00 2001 From: Lukas Oesch Date: Thu, 14 May 2026 15:41:21 +0200 Subject: small refactoring --- kernel/include/kernel/filesystem/dentry.hpp | 2 +- kernel/src/filesystem/dentry.cpp | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/kernel/include/kernel/filesystem/dentry.hpp b/kernel/include/kernel/filesystem/dentry.hpp index 7eef693..3813f61 100644 --- a/kernel/include/kernel/filesystem/dentry.hpp +++ b/kernel/include/kernel/filesystem/dentry.hpp @@ -104,7 +104,7 @@ namespace kernel::filesystem kstd::shared_ptr m_parent; kstd::vector> m_children; kstd::shared_ptr m_inode; - uint32_t m_flags{0}; + uint32_t m_flags; }; } // namespace kernel::filesystem diff --git a/kernel/src/filesystem/dentry.cpp b/kernel/src/filesystem/dentry.cpp index c21771b..7603e11 100644 --- a/kernel/src/filesystem/dentry.cpp +++ b/kernel/src/filesystem/dentry.cpp @@ -17,6 +17,7 @@ namespace kernel::filesystem : m_name(name) , m_parent(parent) , m_inode(inode) + , m_flags(0) { if (!m_inode) { -- cgit v1.2.3 From 13f41e3816bd0be96c9bf728b534a58e6d4e5c28 Mon Sep 17 00:00:00 2001 From: Lukas Oesch Date: Thu, 14 May 2026 15:41:54 +0200 Subject: add todo --- kernel/src/filesystem/mount_table.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/kernel/src/filesystem/mount_table.cpp b/kernel/src/filesystem/mount_table.cpp index 5a49e7a..e0cf140 100644 --- a/kernel/src/filesystem/mount_table.cpp +++ b/kernel/src/filesystem/mount_table.cpp @@ -35,6 +35,7 @@ namespace kernel::filesystem auto mount_table::remove_mount(std::string_view path) -> operation_result { // TODO BA-FS26 check wheter something is open in this mount + // TODO BA-FS26 nearly the same code is in find_exact_mount -> refactor to avoid code duplication auto mount_range = std::ranges::find_last_if(m_mounts, [&](auto const & mount) { return mount->get_mount_path() == path; }); auto mount_it = mount_range.begin(); -- cgit v1.2.3 From 245f47af9362e83235a28f993c89f844886e65c3 Mon Sep 17 00:00:00 2001 From: Marcel Braun Date: Thu, 14 May 2026 16:25:15 +0200 Subject: Unify header inclusion syntax --- kernel/include/kernel/filesystem/vfs.hpp | 2 +- kernel/src/filesystem/ext2/inode.tests.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/kernel/include/kernel/filesystem/vfs.hpp b/kernel/include/kernel/filesystem/vfs.hpp index b5053a2..4b6de53 100644 --- a/kernel/include/kernel/filesystem/vfs.hpp +++ b/kernel/include/kernel/filesystem/vfs.hpp @@ -1,8 +1,8 @@ #ifndef TEACH_OS_KERNEL_FILESYSTEM_VFS_HPP #define TEACH_OS_KERNEL_FILESYSTEM_VFS_HPP -#include "kernel/filesystem/devfs/filesystem.hpp" #include +#include #include #include diff --git a/kernel/src/filesystem/ext2/inode.tests.cpp b/kernel/src/filesystem/ext2/inode.tests.cpp index 45bea51..efc0660 100644 --- a/kernel/src/filesystem/ext2/inode.tests.cpp +++ b/kernel/src/filesystem/ext2/inode.tests.cpp @@ -1,9 +1,9 @@ #include -#include "kernel/filesystem/ext2/superblock.hpp" #include #include #include +#include #include #include #include -- cgit v1.2.3 From c6953852b9e10823830688bdfb269650b080f1bb Mon Sep 17 00:00:00 2001 From: Marcel Braun Date: Thu, 14 May 2026 16:29:29 +0200 Subject: Track dentry instead of inode in open_file_descriptor --- kernel/include/kernel/filesystem/open_file_descriptor.hpp | 12 ++++++------ kernel/kapi/filesystem.cpp | 2 +- kernel/src/filesystem/open_file_descriptor.cpp | 15 ++++++++------- kernel/src/filesystem/open_file_descriptor.tests.cpp | 14 +++++++++----- kernel/src/filesystem/open_file_table.tests.cpp | 10 +++++++--- kernel/src/filesystem/vfs.tests.cpp | 6 +++--- 6 files changed, 34 insertions(+), 25 deletions(-) diff --git a/kernel/include/kernel/filesystem/open_file_descriptor.hpp b/kernel/include/kernel/filesystem/open_file_descriptor.hpp index 036dcf0..823fe13 100644 --- a/kernel/include/kernel/filesystem/open_file_descriptor.hpp +++ b/kernel/include/kernel/filesystem/open_file_descriptor.hpp @@ -1,7 +1,7 @@ #ifndef TEACH_OS_KERNEL_FILESYSTEM_OPEN_FILE_DESCRIPTOR_HPP #define TEACH_OS_KERNEL_FILESYSTEM_OPEN_FILE_DESCRIPTOR_HPP -#include +#include #include @@ -11,15 +11,15 @@ namespace kernel::filesystem { /** @brief Represents an open file descriptor in the filesystem. This class encapsulates the state of an open file, - including a reference to the associated inode and the current file offset. + including a reference to the associated dentry and the current file offset. */ struct open_file_descriptor { /** - @brief Constructs an open file descriptor for the given @p inode. - @param inode The inode to associate with the open file descriptor. + @brief Constructs an open file descriptor for the given @p dentry. + @param dentry The dentry to associate with the open file descriptor. */ - explicit open_file_descriptor(kstd::shared_ptr const & inode); + explicit open_file_descriptor(kstd::shared_ptr const & dentry); /** @brief Destructor for the open file descriptor. @@ -53,7 +53,7 @@ namespace kernel::filesystem [[nodiscard]] auto offset() const -> size_t; private: - kstd::shared_ptr m_inode; + kstd::shared_ptr m_dentry; size_t m_offset; }; diff --git a/kernel/kapi/filesystem.cpp b/kernel/kapi/filesystem.cpp index 4c68f28..77d7eb0 100644 --- a/kernel/kapi/filesystem.cpp +++ b/kernel/kapi/filesystem.cpp @@ -35,7 +35,7 @@ namespace kapi::filesystem { if (auto dentry = kernel::filesystem::vfs::get().open(path)) { - auto open_file_descriptor = kstd::make_shared(dentry->get_inode()); + auto open_file_descriptor = kstd::make_shared(dentry); return kernel::filesystem::open_file_table::get().add_file(open_file_descriptor); } diff --git a/kernel/src/filesystem/open_file_descriptor.cpp b/kernel/src/filesystem/open_file_descriptor.cpp index 25bffbd..27e6449 100644 --- a/kernel/src/filesystem/open_file_descriptor.cpp +++ b/kernel/src/filesystem/open_file_descriptor.cpp @@ -1,6 +1,7 @@ -#include #include +#include + #include #include @@ -8,26 +9,26 @@ namespace kernel::filesystem { - open_file_descriptor::open_file_descriptor(kstd::shared_ptr const & inode) - : m_inode(inode) + open_file_descriptor::open_file_descriptor(kstd::shared_ptr const & dentry) + : m_dentry(dentry) , m_offset(0) { - if (!inode) + if (!dentry) { - kstd::os::panic("[FILESYSTEM] open_file_descriptor constructed with null inode."); + kstd::os::panic("[FILESYSTEM] open_file_descriptor constructed with null dentry."); } } auto open_file_descriptor::read(void * buffer, size_t size) -> size_t { - auto read_bytes = m_inode->read(buffer, m_offset, size); + auto read_bytes = m_dentry->get_inode()->read(buffer, m_offset, size); m_offset += read_bytes; return read_bytes; } auto open_file_descriptor::write(void const * buffer, size_t size) -> size_t { - auto written_bytes = m_inode->write(buffer, m_offset, size); + auto written_bytes = m_dentry->get_inode()->write(buffer, m_offset, size); m_offset += written_bytes; return written_bytes; } diff --git a/kernel/src/filesystem/open_file_descriptor.tests.cpp b/kernel/src/filesystem/open_file_descriptor.tests.cpp index 53835ba..8c24cf0 100644 --- a/kernel/src/filesystem/open_file_descriptor.tests.cpp +++ b/kernel/src/filesystem/open_file_descriptor.tests.cpp @@ -1,5 +1,7 @@ #include +#include +#include #include #include #include @@ -16,10 +18,11 @@ SCENARIO("Open file descriptor construction", "[filesystem][open_file_descriptor]") { - GIVEN("an inode and an open file descriptor for that inode") + GIVEN("a dentry and an open file descriptor for that dentry") { auto inode = kstd::make_shared(); - auto file_descriptor = kernel::filesystem::open_file_descriptor{inode}; + auto dentry = kstd::make_shared(nullptr, inode, "test_dentry"); + auto file_descriptor = kernel::filesystem::open_file_descriptor{dentry}; THEN("the initial offset is zero") { @@ -30,10 +33,11 @@ SCENARIO("Open file descriptor construction", "[filesystem][open_file_descriptor SCENARIO("Open file descriptor read/write offset management", "[filesystem][open_file_descriptor]") { - GIVEN("an inode that tracks read/write calls and an open file descriptor for that inode") + GIVEN("a dentry that tracks read/write calls and an open file descriptor for that dentry") { auto inode = kstd::make_shared(); - auto file_descriptor = kernel::filesystem::open_file_descriptor{inode}; + auto dentry = kstd::make_shared(nullptr, inode, "test_dentry"); + auto file_descriptor = kernel::filesystem::open_file_descriptor{dentry}; THEN("the offset is updated correctly after reads") { @@ -78,7 +82,7 @@ SCENARIO_METHOD(kernel::tests::filesystem::storage_boot_module_vfs_fixture, "Ope auto & vfs = kernel::filesystem::vfs::get(); auto dentry = vfs.open("/information/info_1.txt"); REQUIRE(dentry != nullptr); - auto ofd = kstd::make_shared(dentry->get_inode()); + auto ofd = kstd::make_shared(dentry); THEN("the file can be read and the offset is updated") { diff --git a/kernel/src/filesystem/open_file_table.tests.cpp b/kernel/src/filesystem/open_file_table.tests.cpp index a5c791d..456d6b7 100644 --- a/kernel/src/filesystem/open_file_table.tests.cpp +++ b/kernel/src/filesystem/open_file_table.tests.cpp @@ -1,5 +1,6 @@ #include +#include #include #include @@ -15,8 +16,10 @@ SCENARIO("Open file table add/get file", "[filesystem][open_file_table]") { auto & table = kernel::filesystem::open_file_table::get(); auto inode = kstd::make_shared(); - auto file_descriptor_1 = kstd::make_shared(inode); - auto file_descriptor_2 = kstd::make_shared(inode); + auto dentry = kstd::make_shared(nullptr, inode, "test_dentry"); + + auto file_descriptor_1 = kstd::make_shared(dentry); + auto file_descriptor_2 = kstd::make_shared(dentry); WHEN("adding the open file descriptor to the open file table") { @@ -69,7 +72,8 @@ SCENARIO("Open file table remove file", "[filesystem][open_file_table]") { auto & table = kernel::filesystem::open_file_table::get(); auto inode = kstd::make_shared(); - auto file_descriptor = kstd::make_shared(inode); + auto dentry = kstd::make_shared(nullptr, inode, "test_dentry"); + auto file_descriptor = kstd::make_shared(dentry); auto fd = table.add_file(file_descriptor); WHEN("removing the file descriptor using the file descriptor") diff --git a/kernel/src/filesystem/vfs.tests.cpp b/kernel/src/filesystem/vfs.tests.cpp index 0f1d6d5..eaffbc9 100644 --- a/kernel/src/filesystem/vfs.tests.cpp +++ b/kernel/src/filesystem/vfs.tests.cpp @@ -310,7 +310,7 @@ SCENARIO_METHOD(kernel::tests::filesystem::storage_boot_module_vfs_fixture, "VFS auto dentry = vfs.open("/information/sheep_1.txt"); REQUIRE(dentry != nullptr); - auto sheep_1_ofd = kstd::make_shared(dentry->get_inode()); + auto sheep_1_ofd = kstd::make_shared(dentry); kstd::vector buffer(7); auto bytes_read = sheep_1_ofd->read(buffer.data(), buffer.size()); @@ -335,8 +335,8 @@ SCENARIO_METHOD(kernel::tests::filesystem::storage_boot_module_vfs_fixture, "VFS REQUIRE(sheep_1 != nullptr); REQUIRE(goat_1 != nullptr); - auto sheep_1_ofd = kstd::make_shared(sheep_1->get_inode()); - auto goat_1_ofd = kstd::make_shared(goat_1->get_inode()); + auto sheep_1_ofd = kstd::make_shared(sheep_1); + auto goat_1_ofd = kstd::make_shared(goat_1); kstd::vector sheep_buffer(7); auto bytes_read = sheep_1_ofd->read(sheep_buffer.data(), sheep_buffer.size()); -- cgit v1.2.3 From 216ec44cf2fdc914ce38e3ab56eb3a8d82b54c77 Mon Sep 17 00:00:00 2001 From: Marcel Braun Date: Thu, 14 May 2026 16:59:56 +0200 Subject: Refactor resolve_path --- kernel/include/kernel/filesystem/vfs.hpp | 6 ++++++ kernel/src/filesystem/vfs.cpp | 21 ++++++++++++++++----- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/kernel/include/kernel/filesystem/vfs.hpp b/kernel/include/kernel/filesystem/vfs.hpp index 4b6de53..2aa1dd7 100644 --- a/kernel/include/kernel/filesystem/vfs.hpp +++ b/kernel/include/kernel/filesystem/vfs.hpp @@ -4,11 +4,13 @@ #include #include #include +#include #include #include #include +#include namespace kernel::filesystem { @@ -77,7 +79,11 @@ namespace kernel::filesystem vfs() = default; auto init_internal() -> void; + [[nodiscard]] auto resolve_path_internal(std::string_view path) + -> std::pair, kstd::shared_ptr>; [[nodiscard]] auto resolve_path(std::string_view path) -> kstd::shared_ptr; + [[nodiscard]] auto find_mount(std::string_view path) -> kstd::shared_ptr; + auto do_mount_internal(kstd::shared_ptr const & mount_point_dentry, kstd::shared_ptr const & fs) -> void; diff --git a/kernel/src/filesystem/vfs.cpp b/kernel/src/filesystem/vfs.cpp index f5d57be..52ffcc8 100644 --- a/kernel/src/filesystem/vfs.cpp +++ b/kernel/src/filesystem/vfs.cpp @@ -20,6 +20,7 @@ #include #include #include +#include namespace { @@ -165,11 +166,11 @@ namespace kernel::filesystem } } - auto vfs::resolve_path(std::string_view path) -> kstd::shared_ptr + auto vfs::resolve_path_internal(std::string_view path) -> std::pair, kstd::shared_ptr> { if (!path::is_valid_absolute_path(path)) { - return nullptr; + return {nullptr, nullptr}; } auto current_mount = m_mount_table.find_exact_mount("/"); @@ -225,7 +226,7 @@ namespace kernel::filesystem auto found_inode = current_fs->lookup(current_dentry->get_inode(), part.view()); if (!found_inode) { - return nullptr; + return {nullptr, nullptr}; } next_dentry = kstd::make_shared(current_dentry, found_inode, part.view()); @@ -246,7 +247,7 @@ namespace kernel::filesystem { if (symlink_counter++ > constants::symloop_max) { - return nullptr; + return {nullptr, nullptr}; } kstd::vector buffer(constants::symlink_max_path_length); @@ -269,9 +270,19 @@ namespace kernel::filesystem current_dentry = next_dentry; } + return {current_dentry, current_mount}; + } + + auto vfs::resolve_path(std::string_view path) -> kstd::shared_ptr + { + return resolve_path_internal(path).first; + } - return current_dentry; + auto vfs::find_mount(std::string_view path) -> kstd::shared_ptr + { + return resolve_path_internal(path).second; } + } // namespace kernel::filesystem namespace kernel::tests::filesystem::vfs -- cgit v1.2.3 From f2b46c2d9cd9b1bf4b5ec5f35593ae60b3740d0c Mon Sep 17 00:00:00 2001 From: Marcel Braun Date: Thu, 14 May 2026 17:24:49 +0200 Subject: Document design rationale for resolve_path return type --- kernel/include/kernel/filesystem/vfs.hpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/kernel/include/kernel/filesystem/vfs.hpp b/kernel/include/kernel/filesystem/vfs.hpp index 2aa1dd7..7a6ebf9 100644 --- a/kernel/include/kernel/filesystem/vfs.hpp +++ b/kernel/include/kernel/filesystem/vfs.hpp @@ -79,6 +79,16 @@ namespace kernel::filesystem vfs() = default; auto init_internal() -> void; + /** + * Note: Resolving a dentry requires traversing mount points; since the + * associated 'mount' object is discovered as a byproduct of this + * traversal, we return it alongside the dentry to avoid redundant + * lookups in callers that require mount context. + * + * If only one component is needed, the convenience wrappers can be used: + * - resolve_path() for the dentry only. + * - find_mount() for the mount context only. + */ [[nodiscard]] auto resolve_path_internal(std::string_view path) -> std::pair, kstd::shared_ptr>; [[nodiscard]] auto resolve_path(std::string_view path) -> kstd::shared_ptr; -- cgit v1.2.3 From 146c40b22b834e4bf8d5e1d7256d3071f11d4bf9 Mon Sep 17 00:00:00 2001 From: Marcel Braun Date: Thu, 14 May 2026 17:33:14 +0200 Subject: Rename mount_point_dentry to target_dentry --- kernel/include/kernel/filesystem/vfs.hpp | 2 +- kernel/src/filesystem/vfs.cpp | 18 +++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/kernel/include/kernel/filesystem/vfs.hpp b/kernel/include/kernel/filesystem/vfs.hpp index 7a6ebf9..dfb8f0e 100644 --- a/kernel/include/kernel/filesystem/vfs.hpp +++ b/kernel/include/kernel/filesystem/vfs.hpp @@ -94,7 +94,7 @@ namespace kernel::filesystem [[nodiscard]] auto resolve_path(std::string_view path) -> kstd::shared_ptr; [[nodiscard]] auto find_mount(std::string_view path) -> kstd::shared_ptr; - auto do_mount_internal(kstd::shared_ptr const & mount_point_dentry, kstd::shared_ptr const & fs) + auto do_mount_internal(kstd::shared_ptr const & target_dentry, kstd::shared_ptr const & fs) -> void; auto graft_persistent_device_fs(kstd::shared_ptr const & device_fs) -> void; diff --git a/kernel/src/filesystem/vfs.cpp b/kernel/src/filesystem/vfs.cpp index 52ffcc8..31ffa42 100644 --- a/kernel/src/filesystem/vfs.cpp +++ b/kernel/src/filesystem/vfs.cpp @@ -53,9 +53,9 @@ namespace kernel::filesystem auto device_fs = kstd::make_shared(); device_fs->mount(nullptr); - if (auto dev_mount_point_dentry = resolve_path("/dev")) + if (auto dev_target_dentry = resolve_path("/dev")) { - do_mount_internal(dev_mount_point_dentry, device_fs); + do_mount_internal(dev_target_dentry, device_fs); } else { @@ -98,13 +98,13 @@ namespace kernel::filesystem return operation_result::invalid_path; } - if (auto mount_point_dentry = resolve_path(target)) + if (auto target_dentry = resolve_path(target)) { if (auto source_dentry = resolve_path(source)) { if (auto fs = kernel::filesystem::filesystem::probe_and_mount(source_dentry->get_inode())) { - do_mount_internal(mount_point_dentry, fs); + do_mount_internal(target_dentry, fs); return operation_result::success; } return operation_result::invalid_filesystem; @@ -135,10 +135,10 @@ namespace kernel::filesystem return operation_result::mount_point_not_found; } - auto vfs::do_mount_internal(kstd::shared_ptr const & mount_point_dentry, - kstd::shared_ptr const & fs) -> void + auto vfs::do_mount_internal(kstd::shared_ptr const & target_dentry, kstd::shared_ptr const & fs) + -> void { - auto parent_mount_dentry = mount_point_dentry->find_mount_root_dentry(); + auto parent_mount_dentry = target_dentry->find_mount_root_dentry(); kstd::shared_ptr parent_mount = nullptr; if (parent_mount_dentry) { @@ -146,8 +146,8 @@ namespace kernel::filesystem } auto new_fs_root = - kstd::make_shared(mount_point_dentry->get_parent(), fs->root_inode(), mount_point_dentry->get_name()); - auto new_mount = kstd::make_shared(mount_point_dentry, new_fs_root, fs, parent_mount); + kstd::make_shared(target_dentry->get_parent(), fs->root_inode(), target_dentry->get_name()); + auto new_mount = kstd::make_shared(target_dentry, new_fs_root, fs, parent_mount); m_mount_table.add_mount(new_mount); } -- cgit v1.2.3 From 4cc120e7dba5c858a3a0f68b63e91e8d7b831701 Mon Sep 17 00:00:00 2001 From: Marcel Braun Date: Thu, 14 May 2026 18:03:44 +0200 Subject: Refactor do_mount_internal to use target_mount as parameter --- kernel/include/kernel/filesystem/vfs.hpp | 4 ++-- kernel/src/filesystem/vfs.cpp | 30 ++++++++++++++---------------- 2 files changed, 16 insertions(+), 18 deletions(-) diff --git a/kernel/include/kernel/filesystem/vfs.hpp b/kernel/include/kernel/filesystem/vfs.hpp index dfb8f0e..8515af0 100644 --- a/kernel/include/kernel/filesystem/vfs.hpp +++ b/kernel/include/kernel/filesystem/vfs.hpp @@ -94,8 +94,8 @@ namespace kernel::filesystem [[nodiscard]] auto resolve_path(std::string_view path) -> kstd::shared_ptr; [[nodiscard]] auto find_mount(std::string_view path) -> kstd::shared_ptr; - auto do_mount_internal(kstd::shared_ptr const & target_dentry, kstd::shared_ptr const & fs) - -> void; + auto do_mount_internal(kstd::shared_ptr const & target_dentry, kstd::shared_ptr const & target_mount, + kstd::shared_ptr const & fs) -> void; auto graft_persistent_device_fs(kstd::shared_ptr const & device_fs) -> void; diff --git a/kernel/src/filesystem/vfs.cpp b/kernel/src/filesystem/vfs.cpp index 31ffa42..c395e74 100644 --- a/kernel/src/filesystem/vfs.cpp +++ b/kernel/src/filesystem/vfs.cpp @@ -47,7 +47,8 @@ namespace kernel::filesystem root_fs->mount(nullptr); auto root_fs_root_dentry = kstd::make_shared(nullptr, root_fs->root_inode(), "/"); - m_mount_table.add_mount(kstd::make_shared(nullptr, root_fs_root_dentry, root_fs, nullptr)); + auto root_mount = kstd::make_shared(nullptr, root_fs_root_dentry, root_fs, nullptr); + m_mount_table.add_mount(root_mount); // mount devfs at /dev (inside rootfs, temporary, will be shadowed) auto device_fs = kstd::make_shared(); @@ -55,7 +56,7 @@ namespace kernel::filesystem if (auto dev_target_dentry = resolve_path("/dev")) { - do_mount_internal(dev_target_dentry, device_fs); + do_mount_internal(dev_target_dentry, root_mount, device_fs); } else { @@ -69,7 +70,7 @@ namespace kernel::filesystem { if (auto root_dentry = resolve_path("/")) { - do_mount_internal(root_dentry, boot_root_fs); + do_mount_internal(root_dentry, root_mount, boot_root_fs); graft_persistent_device_fs(device_fs); } } @@ -98,13 +99,15 @@ namespace kernel::filesystem return operation_result::invalid_path; } - if (auto target_dentry = resolve_path(target)) + auto [target_dentry, target_mount] = resolve_path_internal(target); + + if (target_dentry && target_mount) { if (auto source_dentry = resolve_path(source)) { if (auto fs = kernel::filesystem::filesystem::probe_and_mount(source_dentry->get_inode())) { - do_mount_internal(target_dentry, fs); + do_mount_internal(target_dentry, target_mount, fs); return operation_result::success; } return operation_result::invalid_filesystem; @@ -135,25 +138,20 @@ namespace kernel::filesystem return operation_result::mount_point_not_found; } - auto vfs::do_mount_internal(kstd::shared_ptr const & target_dentry, kstd::shared_ptr const & fs) + auto vfs::do_mount_internal(kstd::shared_ptr const & target_dentry, + kstd::shared_ptr const & target_mount, kstd::shared_ptr const & fs) -> void { - auto parent_mount_dentry = target_dentry->find_mount_root_dentry(); - kstd::shared_ptr parent_mount = nullptr; - if (parent_mount_dentry) - { - parent_mount = m_mount_table.find_exact_mount(parent_mount_dentry->get_absolute_path().view()); - } - auto new_fs_root = kstd::make_shared(target_dentry->get_parent(), fs->root_inode(), target_dentry->get_name()); - auto new_mount = kstd::make_shared(target_dentry, new_fs_root, fs, parent_mount); + auto new_mount = kstd::make_shared(target_dentry, new_fs_root, fs, target_mount); m_mount_table.add_mount(new_mount); } auto vfs::graft_persistent_device_fs(kstd::shared_ptr const & device_fs) -> void { - if (auto new_root_dentry = resolve_path("/")) + auto [new_root_dentry, root_mount] = resolve_path_internal("/"); + if (new_root_dentry && root_mount) { auto dev_dentry = new_root_dentry->find_child("dev"); if (!dev_dentry) @@ -162,7 +160,7 @@ namespace kernel::filesystem new_root_dentry->add_child(dev_dentry); } - do_mount_internal(dev_dentry, device_fs); + do_mount_internal(dev_dentry, root_mount, device_fs); } } -- cgit v1.2.3 From 5b97abfc9ce1032a0e42be213906b1abd51355dd Mon Sep 17 00:00:00 2001 From: Marcel Braun Date: Thu, 14 May 2026 18:09:17 +0200 Subject: Remove unneeded functionality --- kernel/include/kernel/filesystem/dentry.hpp | 9 +-------- kernel/src/filesystem/dentry.cpp | 10 ---------- 2 files changed, 1 insertion(+), 18 deletions(-) diff --git a/kernel/include/kernel/filesystem/dentry.hpp b/kernel/include/kernel/filesystem/dentry.hpp index 3813f61..925768a 100644 --- a/kernel/include/kernel/filesystem/dentry.hpp +++ b/kernel/include/kernel/filesystem/dentry.hpp @@ -24,8 +24,7 @@ namespace kernel::filesystem */ enum class dentry_flags : uint32_t { - is_mount_point = 1 << 0, - is_mount_root = 1 << 1, + is_mount_point = 1 << 0 }; /** @@ -61,12 +60,6 @@ namespace kernel::filesystem */ [[nodiscard]] auto get_absolute_path() const -> kstd::string; - /** - @brief traverse parent dentries until dentry with is_mount_root flag is found. - @return The found dentry. - */ - [[nodiscard]] auto find_mount_root_dentry() const -> kstd::shared_ptr; - /** @brief Add a @p child dentry. @param child The child dentry to add. diff --git a/kernel/src/filesystem/dentry.cpp b/kernel/src/filesystem/dentry.cpp index 7603e11..d963ed7 100644 --- a/kernel/src/filesystem/dentry.cpp +++ b/kernel/src/filesystem/dentry.cpp @@ -67,16 +67,6 @@ namespace kernel::filesystem return path; } - auto dentry::find_mount_root_dentry() const -> kstd::shared_ptr - { - auto parent = m_parent; - while (parent && !parent->has_flag(dentry_flags::is_mount_root)) - { - parent = parent->get_parent(); - } - return parent; - } - auto dentry::add_child(kstd::shared_ptr const & child) -> void { m_children.push_back(child); -- cgit v1.2.3 From fe0b38db0f8848da8cf28bb883e01bbbb889dd0a Mon Sep 17 00:00:00 2001 From: Marcel Braun Date: Thu, 14 May 2026 21:02:25 +0200 Subject: Remove unneeded functionality part 2 --- kernel/src/filesystem/mount_table.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/kernel/src/filesystem/mount_table.cpp b/kernel/src/filesystem/mount_table.cpp index e0cf140..4b61fb0 100644 --- a/kernel/src/filesystem/mount_table.cpp +++ b/kernel/src/filesystem/mount_table.cpp @@ -26,10 +26,6 @@ namespace kernel::filesystem { mount_dentry->set_flag(dentry::dentry_flags::is_mount_point); } - if (auto root_dentry = mount->get_root_dentry()) - { - root_dentry->set_flag(dentry::dentry_flags::is_mount_root); - } } auto mount_table::remove_mount(std::string_view path) -> operation_result -- cgit v1.2.3 From 963c926c68aac4606d80743aca8e7b052eee7efe Mon Sep 17 00:00:00 2001 From: Marcel Braun Date: Fri, 15 May 2026 11:42:33 +0200 Subject: Rename mount_table method from find_exact_mount to find_mount --- kernel/include/kernel/filesystem/mount_table.hpp | 2 +- kernel/src/filesystem/mount_table.cpp | 4 ++-- kernel/src/filesystem/mount_table.tests.cpp | 10 +++++----- kernel/src/filesystem/vfs.cpp | 6 +++--- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/kernel/include/kernel/filesystem/mount_table.hpp b/kernel/include/kernel/filesystem/mount_table.hpp index 59b9503..8bebfe2 100644 --- a/kernel/include/kernel/filesystem/mount_table.hpp +++ b/kernel/include/kernel/filesystem/mount_table.hpp @@ -43,7 +43,7 @@ namespace kernel::filesystem @param path The path to match against the mount paths in the table. @return A pointer to the mount with the exact matching path, or a null pointer if no mount matches the path. */ - [[nodiscard]] auto find_exact_mount(std::string_view path) const -> kstd::shared_ptr; + [[nodiscard]] auto find_mount(std::string_view path) const -> kstd::shared_ptr; private: [[nodiscard]] auto has_child_mounts(kstd::shared_ptr const & parent_mount) const -> bool; diff --git a/kernel/src/filesystem/mount_table.cpp b/kernel/src/filesystem/mount_table.cpp index 4b61fb0..78ac727 100644 --- a/kernel/src/filesystem/mount_table.cpp +++ b/kernel/src/filesystem/mount_table.cpp @@ -31,7 +31,7 @@ namespace kernel::filesystem auto mount_table::remove_mount(std::string_view path) -> operation_result { // TODO BA-FS26 check wheter something is open in this mount - // TODO BA-FS26 nearly the same code is in find_exact_mount -> refactor to avoid code duplication + // TODO BA-FS26 nearly the same code is in find_mount -> refactor to avoid code duplication auto mount_range = std::ranges::find_last_if(m_mounts, [&](auto const & mount) { return mount->get_mount_path() == path; }); auto mount_it = mount_range.begin(); @@ -52,7 +52,7 @@ namespace kernel::filesystem return operation_result::removed; } - auto mount_table::find_exact_mount(std::string_view path) const -> kstd::shared_ptr + auto mount_table::find_mount(std::string_view path) const -> kstd::shared_ptr { auto mount_range = std::ranges::find_last_if(m_mounts, [&](auto const & mount) { return mount->get_mount_path() == path; }); diff --git a/kernel/src/filesystem/mount_table.tests.cpp b/kernel/src/filesystem/mount_table.tests.cpp index 4ae8711..f22b25e 100644 --- a/kernel/src/filesystem/mount_table.tests.cpp +++ b/kernel/src/filesystem/mount_table.tests.cpp @@ -58,14 +58,14 @@ SCENARIO("Adding, finding and removing mounts in the mount table", "[filesystem] THEN("finding mounts by exact valid path returns the correct mount") { - REQUIRE(table.find_exact_mount("/") == mount1); - REQUIRE(table.find_exact_mount("/mnt") == mount2); + REQUIRE(table.find_mount("/") == mount1); + REQUIRE(table.find_mount("/mnt") == mount2); } THEN("finding mounts by exact invalid path returns null") { - REQUIRE(table.find_exact_mount("/nonexistent") == nullptr); - REQUIRE(table.find_exact_mount("/mnt/file") == nullptr); + REQUIRE(table.find_mount("/nonexistent") == nullptr); + REQUIRE(table.find_mount("/mnt/file") == nullptr); } THEN("removing a mount that has no child mounts succeeds") @@ -103,7 +103,7 @@ SCENARIO("Adding, finding and removing mounts in the mount table", "[filesystem] THEN("finding mounts by exact valid path returns the correct mount") { - REQUIRE(table.find_exact_mount("/") == mount2); + REQUIRE(table.find_mount("/") == mount2); } THEN("removing the topmost mount with the same path succeeds") diff --git a/kernel/src/filesystem/vfs.cpp b/kernel/src/filesystem/vfs.cpp index c395e74..9b0440d 100644 --- a/kernel/src/filesystem/vfs.cpp +++ b/kernel/src/filesystem/vfs.cpp @@ -171,7 +171,7 @@ namespace kernel::filesystem return {nullptr, nullptr}; } - auto current_mount = m_mount_table.find_exact_mount("/"); + auto current_mount = m_mount_table.find_mount("/"); if (!current_mount) { kapi::system::panic("[FILESYSTEM] no root mount found."); @@ -232,7 +232,7 @@ namespace kernel::filesystem } else if (next_dentry->has_flag(dentry::dentry_flags::is_mount_point)) { - current_mount = m_mount_table.find_exact_mount(next_dentry->get_absolute_path().view()); + current_mount = m_mount_table.find_mount(next_dentry->get_absolute_path().view()); if (!current_mount) { kapi::system::panic("[FILESYSTEM] mount for dentry with mounted flag not found."); @@ -260,7 +260,7 @@ namespace kernel::filesystem if (path::is_valid_absolute_path(symbolic_link_path)) { - current_mount = m_mount_table.find_exact_mount("/"); + current_mount = m_mount_table.find_mount("/"); current_dentry = current_mount->get_root_dentry(); } continue; -- cgit v1.2.3 From 66d0e68376c9ad3e2b13f6ff8d999a0c85bda1a4 Mon Sep 17 00:00:00 2001 From: Lukas Oesch Date: Fri, 15 May 2026 15:47:06 +0200 Subject: renaming --- kernel/include/kernel/filesystem/vfs.hpp | 5 +++-- kernel/src/filesystem/vfs.cpp | 28 ++++++++++++++-------------- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/kernel/include/kernel/filesystem/vfs.hpp b/kernel/include/kernel/filesystem/vfs.hpp index 8515af0..7e2eae7 100644 --- a/kernel/include/kernel/filesystem/vfs.hpp +++ b/kernel/include/kernel/filesystem/vfs.hpp @@ -94,8 +94,9 @@ namespace kernel::filesystem [[nodiscard]] auto resolve_path(std::string_view path) -> kstd::shared_ptr; [[nodiscard]] auto find_mount(std::string_view path) -> kstd::shared_ptr; - auto do_mount_internal(kstd::shared_ptr const & target_dentry, kstd::shared_ptr const & target_mount, - kstd::shared_ptr const & fs) -> void; + auto do_mount_internal(kstd::shared_ptr const & mount_point_dentry, + kstd::shared_ptr const & parent_mount, kstd::shared_ptr const & fs) + -> void; auto graft_persistent_device_fs(kstd::shared_ptr const & device_fs) -> void; diff --git a/kernel/src/filesystem/vfs.cpp b/kernel/src/filesystem/vfs.cpp index 9b0440d..77ae015 100644 --- a/kernel/src/filesystem/vfs.cpp +++ b/kernel/src/filesystem/vfs.cpp @@ -54,9 +54,9 @@ namespace kernel::filesystem auto device_fs = kstd::make_shared(); device_fs->mount(nullptr); - if (auto dev_target_dentry = resolve_path("/dev")) + if (auto dev_mount_point_dentry = resolve_path("/dev")) { - do_mount_internal(dev_target_dentry, root_mount, device_fs); + do_mount_internal(dev_mount_point_dentry, root_mount, device_fs); } else { @@ -99,15 +99,15 @@ namespace kernel::filesystem return operation_result::invalid_path; } - auto [target_dentry, target_mount] = resolve_path_internal(target); + auto [mount_point_dentry, mount_context] = resolve_path_internal(target); - if (target_dentry && target_mount) + if (mount_point_dentry && mount_context) { if (auto source_dentry = resolve_path(source)) { if (auto fs = kernel::filesystem::filesystem::probe_and_mount(source_dentry->get_inode())) { - do_mount_internal(target_dentry, target_mount, fs); + do_mount_internal(mount_point_dentry, mount_context, fs); return operation_result::success; } return operation_result::invalid_filesystem; @@ -138,26 +138,26 @@ namespace kernel::filesystem return operation_result::mount_point_not_found; } - auto vfs::do_mount_internal(kstd::shared_ptr const & target_dentry, - kstd::shared_ptr const & target_mount, kstd::shared_ptr const & fs) + auto vfs::do_mount_internal(kstd::shared_ptr const & mount_point_dentry, + kstd::shared_ptr const & parent_mount, kstd::shared_ptr const & fs) -> void { auto new_fs_root = - kstd::make_shared(target_dentry->get_parent(), fs->root_inode(), target_dentry->get_name()); - auto new_mount = kstd::make_shared(target_dentry, new_fs_root, fs, target_mount); + kstd::make_shared(mount_point_dentry->get_parent(), fs->root_inode(), mount_point_dentry->get_name()); + auto new_mount = kstd::make_shared(mount_point_dentry, new_fs_root, fs, parent_mount); m_mount_table.add_mount(new_mount); } auto vfs::graft_persistent_device_fs(kstd::shared_ptr const & device_fs) -> void { - auto [new_root_dentry, root_mount] = resolve_path_internal("/"); - if (new_root_dentry && root_mount) + auto [root_mount_point_dentry, root_mount] = resolve_path_internal("/"); + if (root_mount_point_dentry && root_mount) { - auto dev_dentry = new_root_dentry->find_child("dev"); + auto dev_dentry = root_mount_point_dentry->find_child("dev"); if (!dev_dentry) { - dev_dentry = kstd::make_shared(new_root_dentry, device_fs->root_inode(), "dev"); - new_root_dentry->add_child(dev_dentry); + dev_dentry = kstd::make_shared(root_mount_point_dentry, device_fs->root_inode(), "dev"); + root_mount_point_dentry->add_child(dev_dentry); } do_mount_internal(dev_dentry, root_mount, device_fs); -- cgit v1.2.3 From 95ff59017db74a6988f791ca9f122254dd743541 Mon Sep 17 00:00:00 2001 From: Lukas Oesch Date: Fri, 15 May 2026 16:17:50 +0200 Subject: refactor find_mount_iterator to avoid code duplication --- kernel/include/kernel/filesystem/mount_table.hpp | 2 ++ kernel/src/filesystem/mount_table.cpp | 17 +++++++++-------- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/kernel/include/kernel/filesystem/mount_table.hpp b/kernel/include/kernel/filesystem/mount_table.hpp index 8bebfe2..742c928 100644 --- a/kernel/include/kernel/filesystem/mount_table.hpp +++ b/kernel/include/kernel/filesystem/mount_table.hpp @@ -47,6 +47,8 @@ namespace kernel::filesystem private: [[nodiscard]] auto has_child_mounts(kstd::shared_ptr const & parent_mount) const -> bool; + [[nodiscard]] auto find_mount_iterator(std::string_view path) const + -> kstd::vector>::const_iterator; kstd::vector> m_mounts; }; diff --git a/kernel/src/filesystem/mount_table.cpp b/kernel/src/filesystem/mount_table.cpp index 78ac727..b582fd9 100644 --- a/kernel/src/filesystem/mount_table.cpp +++ b/kernel/src/filesystem/mount_table.cpp @@ -31,11 +31,7 @@ namespace kernel::filesystem auto mount_table::remove_mount(std::string_view path) -> operation_result { // TODO BA-FS26 check wheter something is open in this mount - // TODO BA-FS26 nearly the same code is in find_mount -> refactor to avoid code duplication - auto mount_range = - std::ranges::find_last_if(m_mounts, [&](auto const & mount) { return mount->get_mount_path() == path; }); - auto mount_it = mount_range.begin(); - + auto mount_it = find_mount_iterator(path); if (mount_it == m_mounts.end()) { return operation_result::mount_not_found; @@ -54,9 +50,14 @@ namespace kernel::filesystem auto mount_table::find_mount(std::string_view path) const -> kstd::shared_ptr { - auto mount_range = - std::ranges::find_last_if(m_mounts, [&](auto const & mount) { return mount->get_mount_path() == path; }); - auto mount_it = mount_range.begin(); + auto mount_it = find_mount_iterator(path); return (mount_it != m_mounts.end()) ? *mount_it : nullptr; } + + auto mount_table::find_mount_iterator(std::string_view path) const + -> kstd::vector>::const_iterator + { + return std::ranges::find_last_if(m_mounts, [&](auto const & mount) { return mount->get_mount_path() == path; }) + .begin(); + } } // namespace kernel::filesystem \ No newline at end of file -- cgit v1.2.3 From 1d647adb1ba20121eeb5c8e4470f48b2e972b3d4 Mon Sep 17 00:00:00 2001 From: Lukas Oesch Date: Fri, 15 May 2026 16:50:55 +0200 Subject: Mount can only be unmounted if no references are present, increment references on open file and decrement on close file --- kernel/include/kernel/filesystem/mount.hpp | 19 ++++++++++++++++- kernel/include/kernel/filesystem/mount_table.hpp | 3 ++- .../kernel/filesystem/open_file_descriptor.hpp | 8 +++++++- kernel/include/kernel/filesystem/vfs.hpp | 8 ++++++++ kernel/kapi/filesystem.cpp | 10 ++++++++- kernel/src/filesystem/mount.cpp | 22 ++++++++++++++++++++ kernel/src/filesystem/mount_table.cpp | 5 ++++- kernel/src/filesystem/open_file_descriptor.cpp | 7 ++++++- .../src/filesystem/open_file_descriptor.tests.cpp | 24 +++++++++++----------- kernel/src/filesystem/vfs.cpp | 17 ++++++++++++++- 10 files changed, 104 insertions(+), 19 deletions(-) diff --git a/kernel/include/kernel/filesystem/mount.hpp b/kernel/include/kernel/filesystem/mount.hpp index f920891..fb5a627 100644 --- a/kernel/include/kernel/filesystem/mount.hpp +++ b/kernel/include/kernel/filesystem/mount.hpp @@ -54,12 +54,29 @@ namespace kernel::filesystem */ [[nodiscard]] auto get_parent_mount() const -> kstd::shared_ptr const &; + /** + @brief Increment the reference count for this mount. + */ + auto increment_ref_count() -> void; + + /** + @brief Decrement the reference count for this mount. + @return True if the reference count reached zero, false otherwise. + */ + auto decrement_ref_count() -> bool; + + /** + @brief Check if the mount is ready to be unmounted. + @return True if the mount is ready to be unmounted, false otherwise. + */ + [[nodiscard]] auto is_ready_to_unmount() const -> bool; + private: kstd::shared_ptr m_mount_dentry; kstd::shared_ptr m_root_dentry; kstd::shared_ptr m_filesystem{}; kstd::shared_ptr m_parent_mount{}; - std::atomic_uint32_t m_ref_count{0}; // TODO BA-FS26 + std::atomic_size_t m_ref_count; }; } // namespace kernel::filesystem diff --git a/kernel/include/kernel/filesystem/mount_table.hpp b/kernel/include/kernel/filesystem/mount_table.hpp index 742c928..4f2d1b7 100644 --- a/kernel/include/kernel/filesystem/mount_table.hpp +++ b/kernel/include/kernel/filesystem/mount_table.hpp @@ -22,7 +22,8 @@ namespace kernel::filesystem { removed = 0, has_child_mounts = -1, - mount_not_found = -2 + mount_not_found = -2, + cannot_be_unmounted = -3 }; /** diff --git a/kernel/include/kernel/filesystem/open_file_descriptor.hpp b/kernel/include/kernel/filesystem/open_file_descriptor.hpp index 823fe13..7ca7350 100644 --- a/kernel/include/kernel/filesystem/open_file_descriptor.hpp +++ b/kernel/include/kernel/filesystem/open_file_descriptor.hpp @@ -50,7 +50,13 @@ namespace kernel::filesystem @brief Returns the current file offset for this open file descriptor. @return The current file offset in bytes. */ - [[nodiscard]] auto offset() const -> size_t; + [[nodiscard]] auto get_offset() const -> size_t; + + /** + @brief Return a reference to the dentry associated with this open file descriptor. + @return A reference to the associated dentry. + */ + [[nodiscard]] auto get_dentry() const -> kstd::shared_ptr const &; private: kstd::shared_ptr m_dentry; diff --git a/kernel/include/kernel/filesystem/vfs.hpp b/kernel/include/kernel/filesystem/vfs.hpp index 7e2eae7..48b99b2 100644 --- a/kernel/include/kernel/filesystem/vfs.hpp +++ b/kernel/include/kernel/filesystem/vfs.hpp @@ -33,6 +33,7 @@ namespace kernel::filesystem mount_point_not_found = -3, unmount_failed = -4, invalid_filesystem = -5, + close_failed = -6 }; /** @@ -60,6 +61,13 @@ namespace kernel::filesystem */ auto open(std::string_view path) -> kstd::shared_ptr; + /** + @brief Close a file by its associated @p dentry. + @param dentry The dentry of the file to close. + @return The result of the close operation. + */ + auto close(kstd::shared_ptr const & dentry) -> operation_result; + /** @brief Mount a @p source path to a specific @p target path. @param source The source of the filesystem to mount. diff --git a/kernel/kapi/filesystem.cpp b/kernel/kapi/filesystem.cpp index 77d7eb0..1782da5 100644 --- a/kernel/kapi/filesystem.cpp +++ b/kernel/kapi/filesystem.cpp @@ -44,7 +44,15 @@ namespace kapi::filesystem auto close(int file_descriptor) -> int { - return kernel::filesystem::open_file_table::get().remove_file(file_descriptor); + if (auto open_file_descriptor = kernel::filesystem::open_file_table::get().get_file(file_descriptor)) + { + if (kernel::filesystem::vfs::get().close(open_file_descriptor->get_dentry()) == + kernel::filesystem::vfs::operation_result::success) + { + return kernel::filesystem::open_file_table::get().remove_file(file_descriptor); + } + } + return -1; } auto read(int file_descriptor, void * buffer, size_t size) -> ssize_t diff --git a/kernel/src/filesystem/mount.cpp b/kernel/src/filesystem/mount.cpp index 749c86a..3016509 100644 --- a/kernel/src/filesystem/mount.cpp +++ b/kernel/src/filesystem/mount.cpp @@ -18,6 +18,7 @@ namespace kernel::filesystem , m_root_dentry(root_dentry) , m_filesystem(fs) , m_parent_mount(parent_mount) + , m_ref_count(0) { if (!m_filesystem) { @@ -53,4 +54,25 @@ namespace kernel::filesystem { return m_parent_mount; } + + auto mount::increment_ref_count() -> void + { + m_ref_count += 1; + } + + auto mount::decrement_ref_count() -> bool + { + if (m_ref_count == 0) + { + return false; + } + + m_ref_count -= 1; + return true; + } + + auto mount::is_ready_to_unmount() const -> bool + { + return m_ref_count == 0; + } } // namespace kernel::filesystem \ No newline at end of file diff --git a/kernel/src/filesystem/mount_table.cpp b/kernel/src/filesystem/mount_table.cpp index b582fd9..9951590 100644 --- a/kernel/src/filesystem/mount_table.cpp +++ b/kernel/src/filesystem/mount_table.cpp @@ -30,7 +30,6 @@ namespace kernel::filesystem auto mount_table::remove_mount(std::string_view path) -> operation_result { - // TODO BA-FS26 check wheter something is open in this mount auto mount_it = find_mount_iterator(path); if (mount_it == m_mounts.end()) { @@ -38,6 +37,10 @@ namespace kernel::filesystem } auto const & mount = *mount_it; + if (!mount->is_ready_to_unmount()) + { + return operation_result::cannot_be_unmounted; + } if (has_child_mounts(mount)) { return operation_result::has_child_mounts; diff --git a/kernel/src/filesystem/open_file_descriptor.cpp b/kernel/src/filesystem/open_file_descriptor.cpp index 27e6449..ebaabef 100644 --- a/kernel/src/filesystem/open_file_descriptor.cpp +++ b/kernel/src/filesystem/open_file_descriptor.cpp @@ -33,9 +33,14 @@ namespace kernel::filesystem return written_bytes; } - auto open_file_descriptor::offset() const -> size_t + auto open_file_descriptor::get_offset() const -> size_t { return m_offset; } + auto open_file_descriptor::get_dentry() const -> kstd::shared_ptr const & + { + return m_dentry; + } + } // namespace kernel::filesystem \ No newline at end of file diff --git a/kernel/src/filesystem/open_file_descriptor.tests.cpp b/kernel/src/filesystem/open_file_descriptor.tests.cpp index 8c24cf0..1910b8b 100644 --- a/kernel/src/filesystem/open_file_descriptor.tests.cpp +++ b/kernel/src/filesystem/open_file_descriptor.tests.cpp @@ -26,7 +26,7 @@ SCENARIO("Open file descriptor construction", "[filesystem][open_file_descriptor THEN("the initial offset is zero") { - REQUIRE(file_descriptor.offset() == 0); + REQUIRE(file_descriptor.get_offset() == 0); } } } @@ -42,29 +42,29 @@ SCENARIO("Open file descriptor read/write offset management", "[filesystem][open THEN("the offset is updated correctly after reads") { REQUIRE(file_descriptor.read(nullptr, 100) == 100); - REQUIRE(file_descriptor.offset() == 100); + REQUIRE(file_descriptor.get_offset() == 100); REQUIRE(file_descriptor.read(nullptr, 50) == 50); - REQUIRE(file_descriptor.offset() == 150); + REQUIRE(file_descriptor.get_offset() == 150); } THEN("the offset is updated correctly after writes") { REQUIRE(file_descriptor.write(nullptr, 200) == 200); - REQUIRE(file_descriptor.offset() == 200); + REQUIRE(file_descriptor.get_offset() == 200); REQUIRE(file_descriptor.write(nullptr, 25) == 25); - REQUIRE(file_descriptor.offset() == 225); + REQUIRE(file_descriptor.get_offset() == 225); } THEN("reads and writes both update the same offset") { REQUIRE(file_descriptor.read(nullptr, 10) == 10); - REQUIRE(file_descriptor.offset() == 10); + REQUIRE(file_descriptor.get_offset() == 10); REQUIRE(file_descriptor.write(nullptr, 20) == 20); - REQUIRE(file_descriptor.offset() == 30); + REQUIRE(file_descriptor.get_offset() == 30); REQUIRE(file_descriptor.read(nullptr, 5) == 5); - REQUIRE(file_descriptor.offset() == 35); + REQUIRE(file_descriptor.get_offset() == 35); REQUIRE(file_descriptor.write(nullptr, 15) == 15); - REQUIRE(file_descriptor.offset() == 50); + REQUIRE(file_descriptor.get_offset() == 50); } } } @@ -89,7 +89,7 @@ SCENARIO_METHOD(kernel::tests::filesystem::storage_boot_module_vfs_fixture, "Ope kstd::vector buffer(32); auto bytes_read = ofd->read(buffer.data(), buffer.size()); REQUIRE(bytes_read == 7); - REQUIRE(ofd->offset() == 7); + REQUIRE(ofd->get_offset() == 7); std::string_view buffer_as_str{reinterpret_cast(buffer.data()), static_cast(bytes_read)}; REQUIRE(buffer_as_str == "info_1\n"); @@ -100,11 +100,11 @@ SCENARIO_METHOD(kernel::tests::filesystem::storage_boot_module_vfs_fixture, "Ope kstd::vector buffer(4); auto bytes_read_1 = ofd->read(buffer.data(), buffer.size() / 2); REQUIRE(bytes_read_1 == buffer.size() / 2); - REQUIRE(ofd->offset() == buffer.size() / 2); + REQUIRE(ofd->get_offset() == buffer.size() / 2); auto bytes_read_2 = ofd->read(buffer.data() + buffer.size() / 2, buffer.size() / 2); REQUIRE(bytes_read_2 == buffer.size() / 2); - REQUIRE(ofd->offset() == buffer.size()); + REQUIRE(ofd->get_offset() == buffer.size()); std::string_view buffer_as_str{reinterpret_cast(buffer.data()), bytes_read_1 + bytes_read_2}; REQUIRE(buffer_as_str == "info"); diff --git a/kernel/src/filesystem/vfs.cpp b/kernel/src/filesystem/vfs.cpp index 77ae015..3b3d6ff 100644 --- a/kernel/src/filesystem/vfs.cpp +++ b/kernel/src/filesystem/vfs.cpp @@ -89,7 +89,22 @@ namespace kernel::filesystem auto vfs::open(std::string_view path) -> kstd::shared_ptr { - return resolve_path(path); + auto [dentry, mount] = resolve_path_internal(path); + mount->increment_ref_count(); + return dentry; + } + + auto vfs::close(kstd::shared_ptr const & dentry) -> operation_result + { + if (auto mount = find_mount(dentry->get_absolute_path().view())) + { + if (mount->decrement_ref_count()) + { + return operation_result::success; + } + return operation_result::close_failed; + } + return operation_result::invalid_path; } auto vfs::do_mount(std::string_view source, std::string_view target) -> operation_result -- cgit v1.2.3 From 0e279db4e1b799c4db0cc7c714d57686e3de7089 Mon Sep 17 00:00:00 2001 From: Lukas Oesch Date: Fri, 15 May 2026 22:36:05 +0200 Subject: refactoring rootfs, no separate child management needed --- .../kernel/filesystem/rootfs/filesystem.hpp | 2 +- kernel/include/kernel/filesystem/rootfs/inode.hpp | 30 +------------- kernel/src/filesystem/rootfs/filesystem.cpp | 9 +---- kernel/src/filesystem/rootfs/filesystem.tests.cpp | 7 ---- kernel/src/filesystem/rootfs/inode.cpp | 19 --------- kernel/src/filesystem/rootfs/inode.tests.cpp | 47 ---------------------- kernel/src/filesystem/vfs.cpp | 10 +---- 7 files changed, 5 insertions(+), 119 deletions(-) diff --git a/kernel/include/kernel/filesystem/rootfs/filesystem.hpp b/kernel/include/kernel/filesystem/rootfs/filesystem.hpp index cc778d8..f99440b 100644 --- a/kernel/include/kernel/filesystem/rootfs/filesystem.hpp +++ b/kernel/include/kernel/filesystem/rootfs/filesystem.hpp @@ -31,7 +31,7 @@ namespace kernel::filesystem::rootfs @brief Looks up an inode by @p name within a @p parent directory. @param parent The parent directory inode. @param name The name of the inode to look up. - @return A pointer to the found inode, or a null pointer if not found. + @return Always returns nullptr. */ auto lookup(kstd::shared_ptr const & parent, std::string_view name) -> kstd::shared_ptr override; diff --git a/kernel/include/kernel/filesystem/rootfs/inode.hpp b/kernel/include/kernel/filesystem/rootfs/inode.hpp index 58035ea..442dc8a 100644 --- a/kernel/include/kernel/filesystem/rootfs/inode.hpp +++ b/kernel/include/kernel/filesystem/rootfs/inode.hpp @@ -8,16 +8,10 @@ #include #include -#include -#include - namespace kernel::filesystem::rootfs { /** - @brief Represents an inode in the rootfs filesystem. This inode represents a directory in the root filesystem and - maintains a list of child inodes corresponding to files and subdirectories within the root directory. The rootfs inode - provides methods for reading and writing data (which are no-ops for the root directory), as well as adding and looking - up child inodes by name. + @brief Represents an inode in the rootfs filesystem. */ struct inode : kernel::filesystem::inode { @@ -38,28 +32,6 @@ namespace kernel::filesystem::rootfs @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; - - /** - @brief Adds a child inode to the rootfs directory inode with the specified @p name. - @param name The name of the child inode. - */ - auto add_child(std::string_view name) -> void; - - /** - @brief Looks up a child inode by @p name. - @param name The name of the child inode to look up. - @return A pointer to the found child inode, or a null pointer if not found. - */ - auto lookup_child(std::string_view name) -> kstd::shared_ptr; - - /** - @brief Check if this inode represents a directory. - @return returns true, since this inode represents the root directory in the rootfs filesystem. - */ - [[nodiscard]] auto is_directory() const -> bool override; - - private: - kstd::vector>> m_children; }; } // namespace kernel::filesystem::rootfs diff --git a/kernel/src/filesystem/rootfs/filesystem.cpp b/kernel/src/filesystem/rootfs/filesystem.cpp index 6187c3c..d49e237 100644 --- a/kernel/src/filesystem/rootfs/filesystem.cpp +++ b/kernel/src/filesystem/rootfs/filesystem.cpp @@ -11,18 +11,13 @@ namespace kernel::filesystem::rootfs { auto filesystem::mount(kstd::shared_ptr const &) -> operation_result { - auto rfs_inode = kstd::make_shared(); - rfs_inode->add_child("dev"); - m_root_inode = rfs_inode; - + m_root_inode = kstd::make_shared(); return operation_result::success; } - auto filesystem::lookup(kstd::shared_ptr const & parent, std::string_view name) + auto filesystem::lookup(kstd::shared_ptr const &, std::string_view) -> kstd::shared_ptr { - if (auto * rfs_inode = static_cast(parent.get())) - return rfs_inode->lookup_child(name); return nullptr; } } // namespace kernel::filesystem::rootfs diff --git a/kernel/src/filesystem/rootfs/filesystem.tests.cpp b/kernel/src/filesystem/rootfs/filesystem.tests.cpp index 81ac9e4..ae320e9 100644 --- a/kernel/src/filesystem/rootfs/filesystem.tests.cpp +++ b/kernel/src/filesystem/rootfs/filesystem.tests.cpp @@ -21,13 +21,6 @@ SCENARIO("Rootfs filesystem mount and lookup", "[filesystem][rootfs][filesystem] REQUIRE(fs.root_inode() != nullptr); } - THEN("looking up the 'dev' directory returns a valid inode") - { - auto dev_inode = fs.lookup(fs.root_inode(), "dev"); - REQUIRE(dev_inode != nullptr); - REQUIRE(dev_inode->is_directory()); - } - THEN("looking up a non-existent directory returns null") { auto non_existent_inode_1 = fs.lookup(fs.root_inode(), ""); diff --git a/kernel/src/filesystem/rootfs/inode.cpp b/kernel/src/filesystem/rootfs/inode.cpp index d099676..2ad4815 100644 --- a/kernel/src/filesystem/rootfs/inode.cpp +++ b/kernel/src/filesystem/rootfs/inode.cpp @@ -5,10 +5,7 @@ #include #include -#include #include -#include -#include namespace kernel::filesystem::rootfs { @@ -21,20 +18,4 @@ namespace kernel::filesystem::rootfs { return 0; } - - auto inode::add_child(std::string_view name) -> void - { - m_children.push_back(std::make_pair(kstd::string{name}, kstd::make_shared())); - } - - auto inode::lookup_child(std::string_view name) -> kstd::shared_ptr - { - 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/filesystem/rootfs/inode.tests.cpp b/kernel/src/filesystem/rootfs/inode.tests.cpp index 7cc217f..f4b634f 100644 --- a/kernel/src/filesystem/rootfs/inode.tests.cpp +++ b/kernel/src/filesystem/rootfs/inode.tests.cpp @@ -6,53 +6,6 @@ #include -SCENARIO("Rootfs inode creation", "[filesystem][rootfs][inode]") -{ - GIVEN("a rootfs inode") - { - auto inode = kernel::filesystem::rootfs::inode{}; - - THEN("the inode has the correct kind") - { - REQUIRE(inode.is_directory()); - REQUIRE_FALSE(inode.is_device()); - REQUIRE_FALSE(inode.is_regular()); - REQUIRE_FALSE(inode.is_symbolic_link()); - } - - THEN("the inode has no children") - { - REQUIRE(inode.lookup_child("child") == nullptr); - } - } -} - -SCENARIO("Rootfs inode child management", "[filesystem][rootfs][inode]") -{ - GIVEN("a rootfs inode") - { - auto inode = kernel::filesystem::rootfs::inode{}; - - WHEN("adding a child inode") - { - inode.add_child("child"); - inode.add_child("another child"); - - THEN("the child can be looked up by name") - { - auto child_inode = inode.lookup_child("child"); - REQUIRE(child_inode != nullptr); - REQUIRE(child_inode->is_directory()); - } - - THEN("looking up a non-existent child returns null") - { - REQUIRE(inode.lookup_child("nonexistent") == nullptr); - } - } - } -} - SCENARIO("Rootfs inode read/write", "[filesystem][rootfs][inode]") { GIVEN("a rootfs inode") diff --git a/kernel/src/filesystem/vfs.cpp b/kernel/src/filesystem/vfs.cpp index 3b3d6ff..67671e2 100644 --- a/kernel/src/filesystem/vfs.cpp +++ b/kernel/src/filesystem/vfs.cpp @@ -53,15 +53,7 @@ namespace kernel::filesystem // mount devfs at /dev (inside rootfs, temporary, will be shadowed) auto device_fs = kstd::make_shared(); device_fs->mount(nullptr); - - if (auto dev_mount_point_dentry = resolve_path("/dev")) - { - do_mount_internal(dev_mount_point_dentry, root_mount, device_fs); - } - else - { - kapi::system::panic("[FILESYSTEM] failed to resolve /dev for initial devfs mount."); - } + graft_persistent_device_fs(device_fs); // mount boot fs at / (shadows rootfs), re-graft devfs if (auto boot_device_dentry = resolve_path("/dev/ram0")) -- cgit v1.2.3 From 1f9fe3cf18b561749cfbdb2db8ab7572ddc40c03 Mon Sep 17 00:00:00 2001 From: Lukas Oesch Date: Fri, 15 May 2026 22:38:24 +0200 Subject: uniform interface for open and close --- kernel/include/kernel/filesystem/vfs.hpp | 6 +++--- kernel/kapi/filesystem.cpp | 2 +- kernel/src/filesystem/vfs.cpp | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/kernel/include/kernel/filesystem/vfs.hpp b/kernel/include/kernel/filesystem/vfs.hpp index 48b99b2..0058d04 100644 --- a/kernel/include/kernel/filesystem/vfs.hpp +++ b/kernel/include/kernel/filesystem/vfs.hpp @@ -62,11 +62,11 @@ namespace kernel::filesystem auto open(std::string_view path) -> kstd::shared_ptr; /** - @brief Close a file by its associated @p dentry. - @param dentry The dentry of the file to close. + @brief Close a file by its associated @p path. + @param path The path to the file to close. @return The result of the close operation. */ - auto close(kstd::shared_ptr const & dentry) -> operation_result; + auto close(std::string_view path) -> operation_result; /** @brief Mount a @p source path to a specific @p target path. diff --git a/kernel/kapi/filesystem.cpp b/kernel/kapi/filesystem.cpp index 1782da5..53a71be 100644 --- a/kernel/kapi/filesystem.cpp +++ b/kernel/kapi/filesystem.cpp @@ -46,7 +46,7 @@ namespace kapi::filesystem { if (auto open_file_descriptor = kernel::filesystem::open_file_table::get().get_file(file_descriptor)) { - if (kernel::filesystem::vfs::get().close(open_file_descriptor->get_dentry()) == + if (kernel::filesystem::vfs::get().close(open_file_descriptor->get_dentry()->get_absolute_path().view()) == kernel::filesystem::vfs::operation_result::success) { return kernel::filesystem::open_file_table::get().remove_file(file_descriptor); diff --git a/kernel/src/filesystem/vfs.cpp b/kernel/src/filesystem/vfs.cpp index 67671e2..de19d38 100644 --- a/kernel/src/filesystem/vfs.cpp +++ b/kernel/src/filesystem/vfs.cpp @@ -86,9 +86,9 @@ namespace kernel::filesystem return dentry; } - auto vfs::close(kstd::shared_ptr const & dentry) -> operation_result + auto vfs::close(std::string_view path) -> operation_result { - if (auto mount = find_mount(dentry->get_absolute_path().view())) + if (auto mount = find_mount(path)) { if (mount->decrement_ref_count()) { -- cgit v1.2.3 From 2396fe6174ec875ba12dc135ab18f84550c07e9a Mon Sep 17 00:00:00 2001 From: Lukas Oesch Date: Fri, 15 May 2026 22:40:52 +0200 Subject: avoid nullptr access --- kernel/src/filesystem/vfs.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/kernel/src/filesystem/vfs.cpp b/kernel/src/filesystem/vfs.cpp index de19d38..f6eae25 100644 --- a/kernel/src/filesystem/vfs.cpp +++ b/kernel/src/filesystem/vfs.cpp @@ -82,6 +82,10 @@ namespace kernel::filesystem auto vfs::open(std::string_view path) -> kstd::shared_ptr { auto [dentry, mount] = resolve_path_internal(path); + if (!dentry || !mount) + { + return nullptr; + } mount->increment_ref_count(); return dentry; } -- cgit v1.2.3 From 16ccdee935d3b14edf93eea5a135e413b2fd47b5 Mon Sep 17 00:00:00 2001 From: Lukas Oesch Date: Fri, 15 May 2026 22:59:23 +0200 Subject: rootfs inode is a directory inode --- kernel/include/kernel/filesystem/rootfs/inode.hpp | 6 ++++++ kernel/src/filesystem/rootfs/inode.cpp | 5 +++++ 2 files changed, 11 insertions(+) diff --git a/kernel/include/kernel/filesystem/rootfs/inode.hpp b/kernel/include/kernel/filesystem/rootfs/inode.hpp index 442dc8a..2671207 100644 --- a/kernel/include/kernel/filesystem/rootfs/inode.hpp +++ b/kernel/include/kernel/filesystem/rootfs/inode.hpp @@ -32,6 +32,12 @@ namespace kernel::filesystem::rootfs @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; + + /** + @brief Check if this inode represents a directory. + @return returns true, since this inode represents the / directory in the rootfs filesystem. + */ + [[nodiscard]] auto is_directory() const -> bool override; }; } // namespace kernel::filesystem::rootfs diff --git a/kernel/src/filesystem/rootfs/inode.cpp b/kernel/src/filesystem/rootfs/inode.cpp index 2ad4815..dbe7948 100644 --- a/kernel/src/filesystem/rootfs/inode.cpp +++ b/kernel/src/filesystem/rootfs/inode.cpp @@ -18,4 +18,9 @@ namespace kernel::filesystem::rootfs { return 0; } + + auto inode::is_directory() const -> bool + { + return true; + } } // namespace kernel::filesystem::rootfs -- cgit v1.2.3 From ba2f62972823df320e05dea7080adf658c2977b3 Mon Sep 17 00:00:00 2001 From: Lukas Oesch Date: Sat, 16 May 2026 13:43:38 +0200 Subject: fix tests, all files must be closed before unmounting --- kernel/src/filesystem/vfs.tests.cpp | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/kernel/src/filesystem/vfs.tests.cpp b/kernel/src/filesystem/vfs.tests.cpp index eaffbc9..28782ec 100644 --- a/kernel/src/filesystem/vfs.tests.cpp +++ b/kernel/src/filesystem/vfs.tests.cpp @@ -105,6 +105,8 @@ SCENARIO_METHOD(kernel::tests::filesystem::storage_boot_module_vfs_fixture, "VFS auto mounted_monkey_1 = vfs.open("/information/monkey_house/monkey_1.txt"); REQUIRE(mounted_monkey_1 != nullptr); + REQUIRE(vfs.close(mounted_monkey_1->get_absolute_path().view()) == + kernel::filesystem::vfs::operation_result::success); REQUIRE(vfs.unmount("/information") == kernel::filesystem::vfs::operation_result::success); auto unmounted_monkey_1 = vfs.open("/information/monkey_house/monkey_1.txt"); @@ -126,6 +128,11 @@ SCENARIO_METHOD(kernel::tests::filesystem::storage_boot_module_vfs_fixture, "VFS REQUIRE(mounted_monkey_1 != nullptr); REQUIRE(mounted_fish1 != nullptr); + REQUIRE(vfs.close(mounted_monkey_1->get_absolute_path().view()) == + kernel::filesystem::vfs::operation_result::success); + REQUIRE(vfs.close(mounted_fish1->get_absolute_path().view()) == + kernel::filesystem::vfs::operation_result::success); + REQUIRE(vfs.unmount("/information") == kernel::filesystem::vfs::operation_result::unmount_failed); REQUIRE(vfs.unmount("/information/monkey_house/infrastructure") == kernel::filesystem::vfs::operation_result::success); @@ -140,6 +147,9 @@ SCENARIO_METHOD(kernel::tests::filesystem::storage_boot_module_vfs_fixture, "VFS auto mounted_tickets = vfs.open("/information/entrance/tickets.txt"); REQUIRE(mounted_tickets != nullptr); + REQUIRE(vfs.close(mounted_tickets->get_absolute_path().view()) == + kernel::filesystem::vfs::operation_result::success); + REQUIRE(vfs.unmount("/information") == kernel::filesystem::vfs::operation_result::success); mounted_tickets = vfs.open("/information/entrance/tickets.txt"); REQUIRE(mounted_tickets == nullptr); @@ -161,6 +171,8 @@ SCENARIO_METHOD(kernel::tests::filesystem::storage_boot_module_vfs_fixture, "VFS auto water = vfs.open("/monkey_house/infrastructure/water.txt"); REQUIRE(water != nullptr); + REQUIRE(vfs.close(water->get_absolute_path().view()) == kernel::filesystem::vfs::operation_result::success); + REQUIRE(vfs.unmount("/") == kernel::filesystem::vfs::operation_result::success); info_1 = vfs.open("/information/info_1.txt"); @@ -180,6 +192,8 @@ SCENARIO_METHOD(kernel::tests::filesystem::storage_boot_module_vfs_fixture, "VFS auto water = vfs.open("/monkey_house/infrastructure/water.txt"); REQUIRE(water != nullptr); + REQUIRE(vfs.close(water->get_absolute_path().view()) == kernel::filesystem::vfs::operation_result::success); + auto dev_ram_16 = vfs.open("/dev/ram16"); REQUIRE(dev_ram_16 == nullptr); @@ -196,6 +210,8 @@ SCENARIO_METHOD(kernel::tests::filesystem::storage_boot_module_vfs_fixture, "VFS auto info_1 = vfs.open("/information/info_1.txt"); REQUIRE(info_1 != nullptr); + REQUIRE(vfs.close(info_1->get_absolute_path().view()) == kernel::filesystem::vfs::operation_result::success); + REQUIRE(vfs.unmount("/dev") == kernel::filesystem::vfs::operation_result::success); REQUIRE(vfs.unmount("/") == kernel::filesystem::vfs::operation_result::success); @@ -317,6 +333,8 @@ SCENARIO_METHOD(kernel::tests::filesystem::storage_boot_module_vfs_fixture, "VFS std::string_view buffer_as_str{reinterpret_cast(buffer.data()), bytes_read}; REQUIRE(buffer_as_str == "sheep_1"); + REQUIRE(vfs.close(dentry->get_absolute_path().view()) == kernel::filesystem::vfs::operation_result::success); + REQUIRE(vfs.unmount("/information") == kernel::filesystem::vfs::operation_result::success); auto unmounted_sheep_1 = vfs.open("/information/sheep_1.txt"); REQUIRE(unmounted_sheep_1 == nullptr); @@ -348,6 +366,9 @@ SCENARIO_METHOD(kernel::tests::filesystem::storage_boot_module_vfs_fixture, "VFS buffer_as_str = std::string_view{reinterpret_cast(goat_buffer.data()), bytes_read}; REQUIRE(buffer_as_str == "goat_1"); + REQUIRE(vfs.close(sheep_1->get_absolute_path().view()) == kernel::filesystem::vfs::operation_result::success); + REQUIRE(vfs.close(goat_1->get_absolute_path().view()) == kernel::filesystem::vfs::operation_result::success); + REQUIRE(vfs.unmount("/information") == kernel::filesystem::vfs::operation_result::unmount_failed); REQUIRE(vfs.unmount("/information/stable") == kernel::filesystem::vfs::operation_result::success); @@ -357,6 +378,9 @@ SCENARIO_METHOD(kernel::tests::filesystem::storage_boot_module_vfs_fixture, "VFS auto still_mounted_sheep_1 = vfs.open("/information/sheep_1.txt"); REQUIRE(still_mounted_sheep_1 != nullptr); + REQUIRE(vfs.close(still_mounted_sheep_1->get_absolute_path().view()) == + kernel::filesystem::vfs::operation_result::success); + REQUIRE(vfs.unmount("/information") == kernel::filesystem::vfs::operation_result::success); auto unmounted_sheep_1 = vfs.open("/information/sheep_1.txt"); REQUIRE(unmounted_sheep_1 == nullptr); -- cgit v1.2.3 From 7ecf092ca7ff91dd59e81eda7ef2b05fe837844d Mon Sep 17 00:00:00 2001 From: Lukas Oesch Date: Sat, 16 May 2026 13:53:34 +0200 Subject: add mount tests --- kernel/include/kernel/filesystem/mount.hpp | 7 +++++ kernel/src/filesystem/mount.cpp | 6 +++++ kernel/src/filesystem/mount.tests.cpp | 42 ++++++++++++++++++++++++++++++ 3 files changed, 55 insertions(+) diff --git a/kernel/include/kernel/filesystem/mount.hpp b/kernel/include/kernel/filesystem/mount.hpp index fb5a627..5d8ea69 100644 --- a/kernel/include/kernel/filesystem/mount.hpp +++ b/kernel/include/kernel/filesystem/mount.hpp @@ -8,6 +8,7 @@ #include #include +#include namespace kernel::filesystem { @@ -71,6 +72,12 @@ namespace kernel::filesystem */ [[nodiscard]] auto is_ready_to_unmount() const -> bool; + /** + @brief Get the current reference count for this mount. + @return The current reference count. + */ + [[nodiscard]] auto get_ref_count() const -> size_t; + private: kstd::shared_ptr m_mount_dentry; kstd::shared_ptr m_root_dentry; diff --git a/kernel/src/filesystem/mount.cpp b/kernel/src/filesystem/mount.cpp index 3016509..1e04083 100644 --- a/kernel/src/filesystem/mount.cpp +++ b/kernel/src/filesystem/mount.cpp @@ -8,6 +8,7 @@ #include #include +#include #include namespace kernel::filesystem @@ -75,4 +76,9 @@ namespace kernel::filesystem { return m_ref_count == 0; } + + auto mount::get_ref_count() const -> size_t + { + return m_ref_count; + } } // namespace kernel::filesystem \ No newline at end of file diff --git a/kernel/src/filesystem/mount.tests.cpp b/kernel/src/filesystem/mount.tests.cpp index 58e9bab..e7dd709 100644 --- a/kernel/src/filesystem/mount.tests.cpp +++ b/kernel/src/filesystem/mount.tests.cpp @@ -29,6 +29,7 @@ SCENARIO("Mount construction", "[filesystem][mount]") REQUIRE(mount.get_root_dentry() == root_dentry); REQUIRE(mount.get_mount_dentry() == root_dentry); REQUIRE(mount.get_mount_path() == "/"); + REQUIRE(mount.is_ready_to_unmount()); } THEN("the mount has no parent mount") @@ -47,3 +48,44 @@ SCENARIO("Mount construction", "[filesystem][mount]") } } } + +SCENARIO("Mount reference counting", "[filesystem][mount]") +{ + GIVEN("a filesystem and a root dentry") + { + auto fs = kstd::make_shared(); + auto root_inode = kstd::make_shared(); + auto root_dentry = kstd::make_shared(nullptr, root_inode, "/"); + + THEN("reference count can be incremented and decremented, the mount is ready to unmount when the reference " + "count == 0") + { + auto mount = kernel::filesystem::mount{root_dentry, root_dentry, fs, nullptr}; + + mount.increment_ref_count(); + REQUIRE(mount.get_ref_count() == 1); + REQUIRE_FALSE(mount.is_ready_to_unmount()); + + mount.increment_ref_count(); + REQUIRE(mount.get_ref_count() == 2); + REQUIRE_FALSE(mount.is_ready_to_unmount()); + + REQUIRE(mount.decrement_ref_count()); + REQUIRE(mount.get_ref_count() == 1); + REQUIRE_FALSE(mount.is_ready_to_unmount()); + + REQUIRE(mount.decrement_ref_count()); + REQUIRE(mount.get_ref_count() == 0); + REQUIRE(mount.is_ready_to_unmount()); + } + + THEN("decrementing reference count when it is already zero does not decrement it below zero") + { + auto mount = kernel::filesystem::mount{root_dentry, root_dentry, fs, nullptr}; + + REQUIRE_FALSE(mount.decrement_ref_count()); + REQUIRE(mount.get_ref_count() == 0); + REQUIRE(mount.is_ready_to_unmount()); + } + } +} \ No newline at end of file -- cgit v1.2.3 From efc7ba748b977a792188724c461852f01c111957 Mon Sep 17 00:00:00 2001 From: Lukas Oesch Date: Sat, 16 May 2026 14:05:49 +0200 Subject: add vfs tests --- kernel/src/filesystem/vfs.cpp | 7 +++---- kernel/src/filesystem/vfs.tests.cpp | 30 ++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/kernel/src/filesystem/vfs.cpp b/kernel/src/filesystem/vfs.cpp index f6eae25..8636d0f 100644 --- a/kernel/src/filesystem/vfs.cpp +++ b/kernel/src/filesystem/vfs.cpp @@ -140,13 +140,12 @@ namespace kernel::filesystem { return operation_result::success; } - - if (remove_result == mount_table::operation_result::has_child_mounts) + else if (remove_result == mount_table::operation_result::mount_not_found) { - return operation_result::unmount_failed; + return operation_result::mount_point_not_found; } - return operation_result::mount_point_not_found; + return operation_result::unmount_failed; } auto vfs::do_mount_internal(kstd::shared_ptr const & mount_point_dentry, diff --git a/kernel/src/filesystem/vfs.tests.cpp b/kernel/src/filesystem/vfs.tests.cpp index 28782ec..648ebb8 100644 --- a/kernel/src/filesystem/vfs.tests.cpp +++ b/kernel/src/filesystem/vfs.tests.cpp @@ -139,6 +139,36 @@ SCENARIO_METHOD(kernel::tests::filesystem::storage_boot_module_vfs_fixture, "VFS REQUIRE(vfs.unmount("/information") == kernel::filesystem::vfs::operation_result::success); } + THEN("image can be mounted, unmount only if no files are open") + { + REQUIRE(vfs.do_mount("/dev/ram16", "/information") == kernel::filesystem::vfs::operation_result::success); + + auto mounted_monkey_1 = vfs.open("/information/monkey_house/monkey_1.txt"); + REQUIRE(mounted_monkey_1 != nullptr); + + REQUIRE(vfs.unmount("/information") == kernel::filesystem::vfs::operation_result::unmount_failed); + + REQUIRE(vfs.close(mounted_monkey_1->get_absolute_path().view()) == + kernel::filesystem::vfs::operation_result::success); + + REQUIRE(vfs.unmount("/information") == kernel::filesystem::vfs::operation_result::success); + } + + THEN("file with invalid path or not opened file cannot be closed") + { + REQUIRE(vfs.close("invalid_path") == kernel::filesystem::vfs::operation_result::invalid_path); + REQUIRE(vfs.close("/information/info_1.txt") == kernel::filesystem::vfs::operation_result::close_failed); + } + + THEN("file cannot be closed twice") + { + auto info_1 = vfs.open("/information/info_1.txt"); + REQUIRE(info_1 != nullptr); + + REQUIRE(vfs.close(info_1->get_absolute_path().view()) == kernel::filesystem::vfs::operation_result::success); + REQUIRE(vfs.close(info_1->get_absolute_path().view()) == kernel::filesystem::vfs::operation_result::close_failed); + } + THEN("images can be stacked mounted and correct file system is unmounted again") { REQUIRE(vfs.do_mount("/dev/ram16", "/information") == kernel::filesystem::vfs::operation_result::success); -- cgit v1.2.3 From 5b40e4a28307eed814adb46188c3f6783651d286 Mon Sep 17 00:00:00 2001 From: Lukas Oesch Date: Sat, 16 May 2026 14:11:49 +0200 Subject: add kapi::filesystem tests --- kernel/kapi/filesystem.tests.cpp | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/kernel/kapi/filesystem.tests.cpp b/kernel/kapi/filesystem.tests.cpp index 1d1f8ee..d241afa 100644 --- a/kernel/kapi/filesystem.tests.cpp +++ b/kernel/kapi/filesystem.tests.cpp @@ -103,6 +103,19 @@ SCENARIO_METHOD(kernel::tests::filesystem::storage_boot_module_vfs_fixture, "Kap REQUIRE(kapi::filesystem::umount("/information") == 0); } + THEN("a filesystem cannot be unmounted if files are still open and can be unmounted after files are closed") + { + REQUIRE(kapi::filesystem::mount("/dev/ram16", "/information") == 0); + + auto fd = kapi::filesystem::open("/information/monkey_house/monkey_1.txt"); + REQUIRE(fd >= 0); + + REQUIRE(kapi::filesystem::umount("/information") < 0); + + REQUIRE(kapi::filesystem::close(fd) == 0); + REQUIRE(kapi::filesystem::umount("/information") == 0); + } + THEN("device can be opened as file and read from") { auto fd = kapi::filesystem::open("/dev/ram0"); @@ -158,6 +171,15 @@ SCENARIO_METHOD(kernel::tests::filesystem::storage_boot_module_vfs_fixture, "Kap REQUIRE(kapi::filesystem::close(999) < 0); } + THEN("same file cannot be closed twice") + { + auto fd = kapi::filesystem::open("/information/info_1.txt"); + REQUIRE(fd >= 0); + + REQUIRE(kapi::filesystem::close(fd) == 0); + REQUIRE(kapi::filesystem::close(fd) < 0); + } + THEN("not opened files cannot be read from") { std::vector buffer(10); -- cgit v1.2.3