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

Refactoring

See merge request teachos/kernel!27

7 files changed, 249 insertions, 251 deletions

diff --git a/kernel/src/filesystem/file_descriptor_table.cpp b/kernel/src/filesystem/file_descriptor_table.cpp
deleted file mode 100644
index db7692b..0000000
--- a/kernel/src/filesystem/file_descriptor_table.cpp
+++ /dev/null
@@ -1,98 +0,0 @@
-#include <kernel/filesystem/file_descriptor_table.hpp>
-
-#include <kernel/filesystem/open_file_description.hpp>
-
-#include <kapi/system.hpp>
-
-#include <kstd/memory>
-
-#include <algorithm>
-#include <cstddef>
-#include <optional>
-
-namespace
-{
-  constinit auto static global_file_descriptor_table = std::optional<kernel::filesystem::file_descriptor_table>{};
-}  // namespace
-
-namespace kernel::filesystem
-{
-  auto file_descriptor_table::init() -> void
-  {
-    if (global_file_descriptor_table)
-    {
-      kapi::system::panic("[FILESYSTEM] File descriptor table has already been initialized.");
-    }
-
-    global_file_descriptor_table.emplace(file_descriptor_table{});
-  }
-
-  auto file_descriptor_table::get() -> file_descriptor_table &
-  {
-    if (!global_file_descriptor_table)
-    {
-      kapi::system::panic("[FILESYSTEM] File descriptor table has not been initialized.");
-    }
-
-    return *global_file_descriptor_table;
-  }
-
-  auto file_descriptor_table::add_file(kstd::shared_ptr<open_file_description> const & file_description) -> int
-  {
-    if (!file_description)
-    {
-      return -1;
-    }
-
-    auto it = std::ranges::find_if(m_open_files, [](auto const & open_file) { return open_file == nullptr; });
-    if (it != m_open_files.end())
-    {
-      *it = file_description;
-      return static_cast<int>(it - m_open_files.begin());
-    }
-
-    m_open_files.push_back(file_description);
-    return static_cast<int>(m_open_files.size() - 1);
-  }
-
-  auto file_descriptor_table::get_file(int fd) const -> kstd::shared_ptr<open_file_description>
-  {
-    if (fd < 0)
-    {
-      return nullptr;
-    }
-
-    auto const index = static_cast<size_t>(fd);
-    if (index >= m_open_files.size())
-    {
-      return nullptr;
-    }
-
-    return m_open_files.at(index);
-  }
-
-  auto file_descriptor_table::remove_file(int fd) -> int
-  {
-    if (fd < 0)
-    {
-      return -1;
-    }
-
-    auto const index = static_cast<size_t>(fd);
-    if (index >= m_open_files.size())
-    {
-      return -1;
-    }
-
-    m_open_files.at(index) = nullptr;
-    return 0;
-  }
-}  // namespace kernel::filesystem
-
-namespace kernel::tests::filesystem::file_descriptor_table
-{
-  auto deinit() -> void
-  {
-    global_file_descriptor_table.reset();
-  }
-}  // namespace kernel::tests::filesystem::file_descriptor_table
diff --git a/kernel/src/filesystem/file_descriptor_table.tests.cpp b/kernel/src/filesystem/file_descriptor_table.tests.cpp
deleted file mode 100644
index 86ed7bf..0000000
--- a/kernel/src/filesystem/file_descriptor_table.tests.cpp
+++ /dev/null
@@ -1,113 +0,0 @@
-#include <kernel/filesystem/file_descriptor_table.hpp>
-
-#include <kernel/filesystem/open_file_description.hpp>
-#include <kernel/test_support/filesystem/inode.hpp>
-
-#include <kstd/memory>
-#include <kstd/print>
-#include <kstd/vector>
-
-#include <catch2/catch_test_macros.hpp>
-
-SCENARIO("File descriptor table add/get file", "[filesystem][file_descriptor_table]")
-{
-  GIVEN("a file descriptor table and an open file description")
-  {
-    auto & table = kernel::filesystem::file_descriptor_table::get();
-    auto inode = kstd::make_shared<kernel::tests::filesystem::inode>();
-    auto file_description_1 = kstd::make_shared<kernel::filesystem::open_file_description>(inode);
-    auto file_description_2 = kstd::make_shared<kernel::filesystem::open_file_description>(inode);
-
-    WHEN("adding the open file description to the file descriptor table")
-    {
-      auto fd_1 = table.add_file(file_description_1);
-      auto fd_2 = table.add_file(file_description_2);
-      auto fd_3 = table.add_file(file_description_2);
-
-      THEN("a valid file descriptor is returned")
-      {
-        REQUIRE(fd_1 == 0);
-        REQUIRE(fd_2 == 1);
-        REQUIRE(fd_3 == 2);
-      }
-
-      THEN("the file description can be retrieved using the returned file descriptor")
-      {
-        auto retrieved_description = table.get_file(fd_1);
-        REQUIRE(retrieved_description == file_description_1);
-      }
-    }
-  }
-
-  GIVEN("a invalid open file description")
-  {
-    auto & table = kernel::filesystem::file_descriptor_table::get();
-
-    THEN("adding a null file description returns an error code")
-    {
-      auto fd = table.add_file(nullptr);
-      REQUIRE(fd == -1);
-    }
-
-    THEN("retrieving a file description with a negative file descriptor returns a null pointer")
-    {
-      auto retrieved_description = table.get_file(-1);
-      REQUIRE(retrieved_description == nullptr);
-    }
-
-    THEN("retrieving a file description with an out-of-bounds file descriptor returns a null pointer")
-    {
-      auto retrieved_description = table.get_file(1000);
-      REQUIRE(retrieved_description == nullptr);
-    }
-  }
-}
-
-SCENARIO("File descriptor table remove file", "[filesystem][file_descriptor_table]")
-{
-  GIVEN("a file descriptor table with an open file description")
-  {
-    auto & table = kernel::filesystem::file_descriptor_table::get();
-    auto inode = kstd::make_shared<kernel::tests::filesystem::inode>();
-    auto file_description = kstd::make_shared<kernel::filesystem::open_file_description>(inode);
-    auto fd = table.add_file(file_description);
-
-    WHEN("removing the file description using the file descriptor")
-    {
-      table.remove_file(fd);
-
-      THEN("the file description can no longer be retrieved using the file descriptor")
-      {
-        auto retrieved_description = table.get_file(fd);
-        REQUIRE(retrieved_description == nullptr);
-      }
-    }
-
-    WHEN("removing a file description the other file descriptor keep the same index")
-    {
-      auto fd2 = table.add_file(file_description);
-      table.remove_file(fd);
-
-      THEN("the second file description can still be retrieved using its file descriptor")
-      {
-        auto retrieved_description = table.get_file(fd2);
-        REQUIRE(retrieved_description == file_description);
-      }
-    }
-  }
-
-  GIVEN("an invalid file descriptor")
-  {
-    auto & table = kernel::filesystem::file_descriptor_table::get();
-
-    THEN("removing a file with a negative file descriptor does nothing")
-    {
-      REQUIRE_NOTHROW(table.remove_file(-1));
-    }
-
-    THEN("removing a file with an out-of-bounds file descriptor does nothing")
-    {
-      REQUIRE_NOTHROW(table.remove_file(1000));
-    }
-  }
-}
\ No newline at end of file
diff --git a/kernel/src/filesystem/open_file_description.cpp b/kernel/src/filesystem/open_file_descriptor.cpp
index 3033e2b..25bffbd 100644
--- a/kernel/src/filesystem/open_file_description.cpp
+++ b/kernel/src/filesystem/open_file_descriptor.cpp
@@ -1,6 +1,5 @@
-#include <kernel/filesystem/open_file_description.hpp>
-
 #include <kernel/filesystem/inode.hpp>
+#include <kernel/filesystem/open_file_descriptor.hpp>
 
 #include <kstd/memory>
 #include <kstd/os/error.hpp>
@@ -9,31 +8,31 @@
 
 namespace kernel::filesystem
 {
-  open_file_description::open_file_description(kstd::shared_ptr<inode> const & inode)
+  open_file_descriptor::open_file_descriptor(kstd::shared_ptr<inode> const & inode)
       : m_inode(inode)
       , m_offset(0)
   {
     if (!inode)
     {
-      kstd::os::panic("[FILESYSTEM] open_file_description constructed with null inode.");
+      kstd::os::panic("[FILESYSTEM] open_file_descriptor constructed with null inode.");
     }
   }
 
-  auto open_file_description::read(void * buffer, size_t size) -> size_t
+  auto open_file_descriptor::read(void * buffer, size_t size) -> size_t
   {
     auto read_bytes = m_inode->read(buffer, m_offset, size);
     m_offset += read_bytes;
     return read_bytes;
   }
 
-  auto open_file_description::write(void const * buffer, size_t size) -> size_t
+  auto open_file_descriptor::write(void const * buffer, size_t size) -> size_t
   {
     auto written_bytes = m_inode->write(buffer, m_offset, size);
     m_offset += written_bytes;
     return written_bytes;
   }
 
-  auto open_file_description::offset() const -> size_t
+  auto open_file_descriptor::offset() const -> size_t
   {
     return m_offset;
   }
diff --git a/kernel/src/filesystem/open_file_description.tests.cpp b/kernel/src/filesystem/open_file_descriptor.tests.cpp
index ce3c81a..095e203 100644
--- a/kernel/src/filesystem/open_file_description.tests.cpp
+++ b/kernel/src/filesystem/open_file_descriptor.tests.cpp
@@ -1,5 +1,4 @@
-#include <kernel/filesystem/open_file_description.hpp>
-
+#include <kernel/filesystem/open_file_descriptor.hpp>
 #include <kernel/filesystem/vfs.hpp>
 #include <kernel/test_support/filesystem/inode.hpp>
 #include <kernel/test_support/filesystem/storage_boot_module_vfs_fixture.hpp>
@@ -14,63 +13,63 @@
 #include <filesystem>
 #include <string_view>
 
-SCENARIO("Open file description construction", "[filesystem][open_file_description]")
+SCENARIO("Open file descriptor construction", "[filesystem][open_file_descriptor]")
 {
-  GIVEN("an inode and an open file description for that inode")
+  GIVEN("an inode and an open file descriptor for that inode")
   {
     auto inode = kstd::make_shared<kernel::tests::filesystem::inode>();
-    auto file_description = kernel::filesystem::open_file_description{inode};
+    auto file_descriptor = kernel::filesystem::open_file_descriptor{inode};
 
     THEN("the initial offset is zero")
     {
-      REQUIRE(file_description.offset() == 0);
+      REQUIRE(file_descriptor.offset() == 0);
     }
   }
 }
 
-SCENARIO("Open file description read/write offset management", "[filesystem][open_file_description]")
+SCENARIO("Open file descriptor read/write offset management", "[filesystem][open_file_descriptor]")
 {
-  GIVEN("an inode that tracks read/write calls and an open file description for that inode")
+  GIVEN("an inode that tracks read/write calls and an open file descriptor for that inode")
   {
     auto inode = kstd::make_shared<kernel::tests::filesystem::inode>();
-    auto file_description = kernel::filesystem::open_file_description{inode};
+    auto file_descriptor = kernel::filesystem::open_file_descriptor{inode};
 
     THEN("the offset is updated correctly after reads")
     {
-      REQUIRE(file_description.read(nullptr, 100) == 100);
-      REQUIRE(file_description.offset() == 100);
-      REQUIRE(file_description.read(nullptr, 50) == 50);
-      REQUIRE(file_description.offset() == 150);
+      REQUIRE(file_descriptor.read(nullptr, 100) == 100);
+      REQUIRE(file_descriptor.offset() == 100);
+      REQUIRE(file_descriptor.read(nullptr, 50) == 50);
+      REQUIRE(file_descriptor.offset() == 150);
     }
 
     THEN("the offset is updated correctly after writes")
     {
-      REQUIRE(file_description.write(nullptr, 200) == 200);
-      REQUIRE(file_description.offset() == 200);
-      REQUIRE(file_description.write(nullptr, 25) == 25);
-      REQUIRE(file_description.offset() == 225);
+      REQUIRE(file_descriptor.write(nullptr, 200) == 200);
+      REQUIRE(file_descriptor.offset() == 200);
+      REQUIRE(file_descriptor.write(nullptr, 25) == 25);
+      REQUIRE(file_descriptor.offset() == 225);
     }
 
     THEN("reads and writes both update the same offset")
     {
-      REQUIRE(file_description.read(nullptr, 10) == 10);
-      REQUIRE(file_description.offset() == 10);
-      REQUIRE(file_description.write(nullptr, 20) == 20);
-      REQUIRE(file_description.offset() == 30);
-      REQUIRE(file_description.read(nullptr, 5) == 5);
-      REQUIRE(file_description.offset() == 35);
-      REQUIRE(file_description.write(nullptr, 15) == 15);
-      REQUIRE(file_description.offset() == 50);
+      REQUIRE(file_descriptor.read(nullptr, 10) == 10);
+      REQUIRE(file_descriptor.offset() == 10);
+      REQUIRE(file_descriptor.write(nullptr, 20) == 20);
+      REQUIRE(file_descriptor.offset() == 30);
+      REQUIRE(file_descriptor.read(nullptr, 5) == 5);
+      REQUIRE(file_descriptor.offset() == 35);
+      REQUIRE(file_descriptor.write(nullptr, 15) == 15);
+      REQUIRE(file_descriptor.offset() == 50);
     }
   }
 }
 
-SCENARIO_METHOD(kernel::tests::filesystem::storage_boot_module_vfs_fixture,
-                "Open file description read with real image", "[filesystem][open_file_description][img]")
+SCENARIO_METHOD(kernel::tests::filesystem::storage_boot_module_vfs_fixture, "Open file descriptor read with real image",
+                "[filesystem][open_file_descriptor][img]")
 {
   auto const image_path = std::filesystem::path{KERNEL_TEST_ASSETS_DIR} / "ext2_1KB_fs.img";
 
-  GIVEN("an open file description for a file in a real image")
+  GIVEN("an open file descriptor for a file in a real image")
   {
     REQUIRE(std::filesystem::exists(image_path));
     REQUIRE_NOTHROW(setup_modules_from_img_and_init_vfs({"test_img_module"}, {image_path}));
@@ -78,7 +77,7 @@ SCENARIO_METHOD(kernel::tests::filesystem::storage_boot_module_vfs_fixture,
     auto & vfs = kernel::filesystem::vfs::get();
     auto dentry = vfs.open("/information/info_1.txt");
     REQUIRE(dentry != nullptr);
-    auto ofd = kstd::make_shared<kernel::filesystem::open_file_description>(dentry->get_inode());
+    auto ofd = kstd::make_shared<kernel::filesystem::open_file_descriptor>(dentry->get_inode());
 
     THEN("the file can be read and the offset is updated")
     {
diff --git a/kernel/src/filesystem/open_file_table.cpp b/kernel/src/filesystem/open_file_table.cpp
new file mode 100644
index 0000000..e47d229
--- /dev/null
+++ b/kernel/src/filesystem/open_file_table.cpp
@@ -0,0 +1,98 @@
+#include <kernel/filesystem/open_file_table.hpp>
+
+#include <kernel/filesystem/open_file_descriptor.hpp>
+
+#include <kapi/system.hpp>
+
+#include <kstd/memory>
+
+#include <algorithm>
+#include <cstddef>
+#include <optional>
+
+namespace
+{
+  constinit auto static global_open_file_table = std::optional<kernel::filesystem::open_file_table>{};
+}  // namespace
+
+namespace kernel::filesystem
+{
+  auto open_file_table::init() -> void
+  {
+    if (global_open_file_table)
+    {
+      kapi::system::panic("[FILESYSTEM] Open file table has already been initialized.");
+    }
+
+    global_open_file_table.emplace(open_file_table{});
+  }
+
+  auto open_file_table::get() -> open_file_table &
+  {
+    if (!global_open_file_table)
+    {
+      kapi::system::panic("[FILESYSTEM] Open file table has not been initialized.");
+    }
+
+    return *global_open_file_table;
+  }
+
+  auto open_file_table::add_file(kstd::shared_ptr<open_file_descriptor> const & file_descriptor) -> int
+  {
+    if (!file_descriptor)
+    {
+      return -1;
+    }
+
+    auto it = std::ranges::find_if(m_open_files, [](auto const & open_file) { return open_file == nullptr; });
+    if (it != m_open_files.end())
+    {
+      *it = file_descriptor;
+      return static_cast<int>(it - m_open_files.begin());
+    }
+
+    m_open_files.push_back(file_descriptor);
+    return static_cast<int>(m_open_files.size() - 1);
+  }
+
+  auto open_file_table::get_file(int fd) const -> kstd::shared_ptr<open_file_descriptor>
+  {
+    if (fd < 0)
+    {
+      return nullptr;
+    }
+
+    auto const index = static_cast<size_t>(fd);
+    if (index >= m_open_files.size())
+    {
+      return nullptr;
+    }
+
+    return m_open_files.at(index);
+  }
+
+  auto open_file_table::remove_file(int fd) -> int
+  {
+    if (fd < 0)
+    {
+      return -1;
+    }
+
+    auto const index = static_cast<size_t>(fd);
+    if (index >= m_open_files.size())
+    {
+      return -1;
+    }
+
+    m_open_files.at(index) = nullptr;
+    return 0;
+  }
+}  // namespace kernel::filesystem
+
+namespace kernel::tests::filesystem::open_file_table
+{
+  auto deinit() -> void
+  {
+    global_open_file_table.reset();
+  }
+}  // namespace kernel::tests::filesystem::open_file_table
diff --git a/kernel/src/filesystem/open_file_table.tests.cpp b/kernel/src/filesystem/open_file_table.tests.cpp
new file mode 100644
index 0000000..a5c791d
--- /dev/null
+++ b/kernel/src/filesystem/open_file_table.tests.cpp
@@ -0,0 +1,113 @@
+#include <kernel/filesystem/open_file_table.hpp>
+
+#include <kernel/filesystem/open_file_descriptor.hpp>
+#include <kernel/test_support/filesystem/inode.hpp>
+
+#include <kstd/memory>
+#include <kstd/print>
+#include <kstd/vector>
+
+#include <catch2/catch_test_macros.hpp>
+
+SCENARIO("Open file table add/get file", "[filesystem][open_file_table]")
+{
+  GIVEN("a open file table and an open file descriptor")
+  {
+    auto & table = kernel::filesystem::open_file_table::get();
+    auto inode = kstd::make_shared<kernel::tests::filesystem::inode>();
+    auto file_descriptor_1 = kstd::make_shared<kernel::filesystem::open_file_descriptor>(inode);
+    auto file_descriptor_2 = kstd::make_shared<kernel::filesystem::open_file_descriptor>(inode);
+
+    WHEN("adding the open file descriptor to the open file table")
+    {
+      auto fd_1 = table.add_file(file_descriptor_1);
+      auto fd_2 = table.add_file(file_descriptor_2);
+      auto fd_3 = table.add_file(file_descriptor_2);
+
+      THEN("a valid file descriptor is returned")
+      {
+        REQUIRE(fd_1 == 0);
+        REQUIRE(fd_2 == 1);
+        REQUIRE(fd_3 == 2);
+      }
+
+      THEN("the file descriptor can be retrieved using the returned file descriptor")
+      {
+        auto retrieved_descriptor = table.get_file(fd_1);
+        REQUIRE(retrieved_descriptor == file_descriptor_1);
+      }
+    }
+  }
+
+  GIVEN("a invalid open file descriptor")
+  {
+    auto & table = kernel::filesystem::open_file_table::get();
+
+    THEN("adding a null file descriptor returns an error code")
+    {
+      auto fd = table.add_file(nullptr);
+      REQUIRE(fd == -1);
+    }
+
+    THEN("retrieving a file descriptor with a negative file descriptor returns a null pointer")
+    {
+      auto retrieved_descriptor = table.get_file(-1);
+      REQUIRE(retrieved_descriptor == nullptr);
+    }
+
+    THEN("retrieving a file descriptor with an out-of-bounds file descriptor returns a null pointer")
+    {
+      auto retrieved_descriptor = table.get_file(1000);
+      REQUIRE(retrieved_descriptor == nullptr);
+    }
+  }
+}
+
+SCENARIO("Open file table remove file", "[filesystem][open_file_table]")
+{
+  GIVEN("a open file table with an open file descriptor")
+  {
+    auto & table = kernel::filesystem::open_file_table::get();
+    auto inode = kstd::make_shared<kernel::tests::filesystem::inode>();
+    auto file_descriptor = kstd::make_shared<kernel::filesystem::open_file_descriptor>(inode);
+    auto fd = table.add_file(file_descriptor);
+
+    WHEN("removing the file descriptor using the file descriptor")
+    {
+      table.remove_file(fd);
+
+      THEN("the file descriptor can no longer be retrieved using the file descriptor")
+      {
+        auto retrieved_descriptor = table.get_file(fd);
+        REQUIRE(retrieved_descriptor == nullptr);
+      }
+    }
+
+    WHEN("removing a file descriptor the other file descriptor keep the same index")
+    {
+      auto fd2 = table.add_file(file_descriptor);
+      table.remove_file(fd);
+
+      THEN("the second file descriptor can still be retrieved using its file descriptor")
+      {
+        auto retrieved_descriptor = table.get_file(fd2);
+        REQUIRE(retrieved_descriptor == file_descriptor);
+      }
+    }
+  }
+
+  GIVEN("an invalid file descriptor")
+  {
+    auto & table = kernel::filesystem::open_file_table::get();
+
+    THEN("removing a file with a negative file descriptor does nothing")
+    {
+      REQUIRE_NOTHROW(table.remove_file(-1));
+    }
+
+    THEN("removing a file with an out-of-bounds file descriptor does nothing")
+    {
+      REQUIRE_NOTHROW(table.remove_file(1000));
+    }
+  }
+}
\ No newline at end of file
diff --git a/kernel/src/filesystem/vfs.tests.cpp b/kernel/src/filesystem/vfs.tests.cpp
index 9cadb4d..979ea42 100644
--- a/kernel/src/filesystem/vfs.tests.cpp
+++ b/kernel/src/filesystem/vfs.tests.cpp
@@ -1,6 +1,6 @@
 #include <kernel/filesystem/vfs.hpp>
 
-#include <kernel/filesystem/open_file_description.hpp>
+#include <kernel/filesystem/open_file_descriptor.hpp>
 #include <kernel/test_support/filesystem/storage_boot_module_vfs_fixture.hpp>
 
 #include <kstd/memory>
@@ -185,7 +185,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<kernel::filesystem::open_file_description>(dentry->get_inode());
+      auto sheep_1_ofd = kstd::make_shared<kernel::filesystem::open_file_descriptor>(dentry->get_inode());
 
       kstd::vector<std::byte> buffer(7);
       auto bytes_read = sheep_1_ofd->read(buffer.data(), buffer.size());
@@ -210,8 +210,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<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());
+      auto sheep_1_ofd = kstd::make_shared<kernel::filesystem::open_file_descriptor>(sheep_1->get_inode());
+      auto goat_1_ofd = kstd::make_shared<kernel::filesystem::open_file_descriptor>(goat_1->get_inode());
 
       kstd::vector<std::byte> sheep_buffer(7);
       auto bytes_read = sheep_1_ofd->read(sheep_buffer.data(), sheep_buffer.size());