Merge branch 'refactoring' into 'develop-BA-FS26'

Refactoring

See merge request teachos/kernel!26

9 files changed, 39 insertions, 25 deletions

diff --git a/kernel/include/kernel/filesystem/devfs/filesystem.hpp b/kernel/include/kernel/filesystem/devfs/filesystem.hpp
index 3a52403..53bb87d 100644
--- a/kernel/include/kernel/filesystem/devfs/filesystem.hpp
+++ b/kernel/include/kernel/filesystem/devfs/filesystem.hpp
@@ -1,6 +1,7 @@
 #ifndef TEACH_OS_KERNEL_FILESYSTEM_DEVFS_FILESYSTEM_HPP
 #define TEACH_OS_KERNEL_FILESYSTEM_DEVFS_FILESYSTEM_HPP
 
+#include "kernel/filesystem/device_inode.hpp"
 #include "kernel/filesystem/filesystem.hpp"
 #include "kernel/filesystem/inode.hpp"
 
@@ -37,6 +38,8 @@ namespace kernel::filesystem::devfs
 
   private:
     auto build_device_inode_table() -> void;
+
+    kstd::vector<kstd::shared_ptr<device_inode>> m_inodes{};
   };
 }  // namespace kernel::filesystem::devfs
 
diff --git a/kernel/include/kernel/filesystem/filesystem.hpp b/kernel/include/kernel/filesystem/filesystem.hpp
index 099caee..16c07ad 100644
--- a/kernel/include/kernel/filesystem/filesystem.hpp
+++ b/kernel/include/kernel/filesystem/filesystem.hpp
@@ -71,7 +71,6 @@ namespace kernel::filesystem
   protected:
     kstd::shared_ptr<inode> m_root_inode{};
     kstd::shared_ptr<inode> m_backing_inode{};
-    kstd::vector<kstd::shared_ptr<inode>> m_inodes{};
   };
 
 }  // namespace kernel::filesystem
diff --git a/kernel/include/kernel/filesystem/vfs.hpp b/kernel/include/kernel/filesystem/vfs.hpp
index 2a9d5f7..678e645 100644
--- a/kernel/include/kernel/filesystem/vfs.hpp
+++ b/kernel/include/kernel/filesystem/vfs.hpp
@@ -52,11 +52,11 @@ namespace kernel::filesystem
     ~vfs() = default;
 
     /**
-    @brief Open a file by its @p path. This method resolves the path and creates an open file description.
+    @brief Open a file by its @p path. This method resolves the path and returns the corresponding dentry.
     @param path The path to the file to open.
-    @return A shared pointer to the open file description or a null pointer if the file could not be opened.
+    @return A shared pointer to the dentry or a null pointer if the file could not be opened.
     */
-    auto open(std::string_view path) -> kstd::shared_ptr<open_file_description>;
+    auto open(std::string_view path) -> kstd::shared_ptr<dentry>;
 
     /**
     @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 30201b7..d3aa617 100644
--- a/kernel/kapi/filesystem.cpp
+++ b/kernel/kapi/filesystem.cpp
@@ -1,8 +1,11 @@
 #include "kapi/filesystem.hpp"
 
 #include "kernel/filesystem/file_descriptor_table.hpp"
+#include "kernel/filesystem/open_file_description.hpp"
 #include "kernel/filesystem/vfs.hpp"
 
+#include <kstd/memory>
+
 #include <cstddef>
 #include <string_view>
 #include <sys/types.h>
@@ -29,8 +32,9 @@ namespace kapi::filesystem
 
   auto open(std::string_view path) -> int
   {
-    if (auto open_file_description = kernel::filesystem::vfs::get().open(path))
+    if (auto dentry = kernel::filesystem::vfs::get().open(path))
     {
+      auto open_file_description = kstd::make_shared<kernel::filesystem::open_file_description>(dentry->get_inode());
       return kernel::filesystem::file_descriptor_table::get().add_file(open_file_description);
     }
 
diff --git a/kernel/src/filesystem/ext2/inode.cpp b/kernel/src/filesystem/ext2/inode.cpp
index bf3f0cf..07a5525 100644
--- a/kernel/src/filesystem/ext2/inode.cpp
+++ b/kernel/src/filesystem/ext2/inode.cpp
@@ -31,7 +31,9 @@ namespace kernel::filesystem::ext2
     while (bytes_read < size)
     {
       auto const block_number = m_filesystem->map_inode_block_index_to_global_block_number(block_index, m_data);
-      if (block_number == 0)  // TODO BA-FS26 really correct?
+      // TODO BA-FS26 really correct? sparse files -> 0 means a full block with zeros --> function
+      // map_inode_block_index_to_global_block_number should return 0 if not possible to find an block
+      if (block_number == 0)
       {
         break;
       }
diff --git a/kernel/src/filesystem/open_file_description.tests.cpp b/kernel/src/filesystem/open_file_description.tests.cpp
index db8eb49..ec35546 100644
--- a/kernel/src/filesystem/open_file_description.tests.cpp
+++ b/kernel/src/filesystem/open_file_description.tests.cpp
@@ -76,8 +76,9 @@ SCENARIO_METHOD(kernel::tests::filesystem::storage_boot_module_vfs_fixture,
     REQUIRE_NOTHROW(setup_modules_from_img_and_init_vfs({"test_img_module"}, {image_path}));
 
     auto & vfs = kernel::filesystem::vfs::get();
-    auto ofd = vfs.open("/information/info_1.txt");
-    REQUIRE(ofd != nullptr);
+    auto dentry = vfs.open("/information/info_1.txt");
+    REQUIRE(dentry != nullptr);
+    auto ofd = kstd::make_shared<kernel::filesystem::open_file_description>(dentry->get_inode());
 
     THEN("the file can be read and the offset is updated")
     {
diff --git a/kernel/src/filesystem/vfs.cpp b/kernel/src/filesystem/vfs.cpp
index 394e926..84c8047 100644
--- a/kernel/src/filesystem/vfs.cpp
+++ b/kernel/src/filesystem/vfs.cpp
@@ -7,7 +7,6 @@
 #include "kernel/filesystem/filesystem.hpp"
 #include "kernel/filesystem/mount.hpp"
 #include "kernel/filesystem/mount_table.hpp"
-#include "kernel/filesystem/open_file_description.hpp"
 #include "kernel/filesystem/rootfs/filesystem.hpp"
 
 #include <kstd/memory>
@@ -65,14 +64,9 @@ namespace kernel::filesystem
     return *active_vfs;
   }
 
-  auto vfs::open(std::string_view path) -> kstd::shared_ptr<open_file_description>
+  auto vfs::open(std::string_view path) -> kstd::shared_ptr<dentry>
   {
-    if (auto dentry = resolve_path(path))
-    {
-      return kstd::make_shared<open_file_description>(dentry->get_inode());
-    }
-
-    return nullptr;
+    return resolve_path(path);
   }
 
   auto vfs::do_mount(std::string_view source, std::string_view target) -> operation_result
diff --git a/kernel/src/filesystem/vfs.tests.cpp b/kernel/src/filesystem/vfs.tests.cpp
index 12dce84..eba157d 100644
--- a/kernel/src/filesystem/vfs.tests.cpp
+++ b/kernel/src/filesystem/vfs.tests.cpp
@@ -1,7 +1,9 @@
 #include "kernel/filesystem/vfs.hpp"
 
+#include "kernel/filesystem/open_file_description.hpp"
 #include "kernel/test_support/filesystem/storage_boot_module_vfs_fixture.hpp"
 
+#include <kstd/memory>
 #include <kstd/vector>
 
 #include <catch2/catch_test_macros.hpp>
@@ -178,11 +180,15 @@ SCENARIO_METHOD(kernel::tests::filesystem::storage_boot_module_vfs_fixture, "VFS
       auto & vfs = kernel::filesystem::vfs::get();
       REQUIRE(vfs.do_mount("/archiv/2024.img", "/information") == kernel::filesystem::vfs::operation_result::success);
 
-      auto sheep_1 = vfs.open("/information/sheep_1.txt");
-      REQUIRE(sheep_1 != nullptr);
+      auto info_1 = vfs.open("/information/info_1.txt");
+      REQUIRE(info_1 == nullptr);
+
+      auto dentry = vfs.open("/information/sheep_1.txt");
+      REQUIRE(dentry != nullptr);
+      auto sheep_1_ofd = kstd::make_shared<kernel::filesystem::open_file_description>(dentry->get_inode());
 
       kstd::vector<std::byte> buffer(7);
-      auto bytes_read = sheep_1->read(buffer.data(), buffer.size());
+      auto bytes_read = sheep_1_ofd->read(buffer.data(), buffer.size());
       std::string_view buffer_as_str{reinterpret_cast<char *>(buffer.data()), bytes_read};
       REQUIRE(buffer_as_str == "sheep_1");
 
@@ -204,13 +210,16 @@ 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<kernel::filesystem::open_file_description>(sheep_1->get_inode());
+      auto goat_1_ofd = kstd::make_shared<kernel::filesystem::open_file_description>(goat_1->get_inode());
+
       kstd::vector<std::byte> sheep_buffer(7);
-      auto bytes_read = sheep_1->read(sheep_buffer.data(), sheep_buffer.size());
+      auto bytes_read = sheep_1_ofd->read(sheep_buffer.data(), sheep_buffer.size());
       std::string_view buffer_as_str{reinterpret_cast<char *>(sheep_buffer.data()), bytes_read};
       REQUIRE(buffer_as_str == "sheep_1");
 
       kstd::vector<std::byte> goat_buffer(6);
-      bytes_read = goat_1->read(goat_buffer.data(), goat_buffer.size());
+      bytes_read = goat_1_ofd->read(goat_buffer.data(), goat_buffer.size());
       buffer_as_str = std::string_view{reinterpret_cast<char *>(goat_buffer.data()), bytes_read};
       REQUIRE(buffer_as_str == "goat_1");
 
diff --git a/kernel/src/main.cpp b/kernel/src/main.cpp
index e296bd5..aee3579 100644
--- a/kernel/src/main.cpp
+++ b/kernel/src/main.cpp
@@ -88,13 +88,14 @@ auto test_file_description_manually() -> void
 auto test_device_with_vfs() -> void
 {
   auto vfs = kernel::filesystem::vfs::get();
-  auto ofd = vfs.open("/dev/ram0");
-  if (!ofd)
+  auto dentry = vfs.open("/dev/ram0");
+  if (!dentry)
   {
     kstd::os::panic("test code failed");
   }
 
   auto fd_table = kernel::filesystem::file_descriptor_table::get();
+  auto ofd = kstd::make_shared<kernel::filesystem::open_file_description>(dentry->get_inode());
   auto fd = fd_table.add_file(ofd);
   kstd::vector<std::byte> buffer{2};
   auto file = fd_table.get_file(fd);
@@ -113,13 +114,14 @@ auto test_file_lookup() -> void
   auto vfs = kernel::filesystem::vfs::get();
   auto read_and_write_file = [&vfs](std::string_view path) {
     kstd::println("[TEST] Reading and writing file at path: {}", path);
-    auto ofd = vfs.open(path);
-    if (!ofd)
+    auto dentry = vfs.open(path);
+    if (!dentry)
     {
       kstd::os::panic("test code failed");
     }
 
     kstd::vector<std::byte> buffer{32};
+    auto ofd = kstd::make_shared<kernel::filesystem::open_file_description>(dentry->get_inode());
     auto number_of_read_bytes = ofd->read(buffer.data(), buffer.size());
     kstd::println("read bytes: {}", number_of_read_bytes);
     kstd::println("buffer: {::#04x}", buffer);