From 03788775fbebbb3669c115b7653af59194b167d3 Mon Sep 17 00:00:00 2001 From: Felix Morgner Date: Wed, 26 Aug 2026 19:12:56 +0200 Subject: kernel/fs: vfs: make file descriptor polymorphic --- kernel/CMakeLists.txt | 2 + kernel/kapi/filesystem.cpp | 17 +- .../filesystem/byte_offset_file_descriptor.cpp | 83 ++++++++ .../filesystem/byte_offset_file_descriptor.hpp | 41 ++++ .../byte_offset_file_descriptor.tests.cpp | 219 ++++++++++++++++++++ .../filesystem/directory_file_descriptor.cpp | 34 ++++ .../filesystem/directory_file_descriptor.hpp | 35 ++++ .../filesystem/directory_file_descriptor.tests.cpp | 81 ++++++++ kernel/kernel/filesystem/open_file_descriptor.cpp | 65 +----- kernel/kernel/filesystem/open_file_descriptor.hpp | 18 +- .../filesystem/open_file_descriptor.tests.cpp | 223 ++++++--------------- kernel/kernel/filesystem/vfs.tests.cpp | 7 +- 12 files changed, 590 insertions(+), 235 deletions(-) create mode 100644 kernel/kernel/filesystem/byte_offset_file_descriptor.cpp create mode 100644 kernel/kernel/filesystem/byte_offset_file_descriptor.hpp create mode 100644 kernel/kernel/filesystem/byte_offset_file_descriptor.tests.cpp create mode 100644 kernel/kernel/filesystem/directory_file_descriptor.cpp create mode 100644 kernel/kernel/filesystem/directory_file_descriptor.hpp create mode 100644 kernel/kernel/filesystem/directory_file_descriptor.tests.cpp (limited to 'kernel') diff --git a/kernel/CMakeLists.txt b/kernel/CMakeLists.txt index 35c3a957..c44383b8 100644 --- a/kernel/CMakeLists.txt +++ b/kernel/CMakeLists.txt @@ -57,9 +57,11 @@ target_sources("kernel_lib" PRIVATE "kernel/drivers/storage/ram_disk.cpp" # Filesystem Subsystem + "kernel/filesystem/byte_offset_file_descriptor.cpp" "kernel/filesystem/dentry.cpp" "kernel/filesystem/device_inode.cpp" "kernel/filesystem/device_number_registry.cpp" + "kernel/filesystem/directory_file_descriptor.cpp" "kernel/filesystem/driver_registry.cpp" "kernel/filesystem/error.cpp" "kernel/filesystem/inode.cpp" diff --git a/kernel/kapi/filesystem.cpp b/kernel/kapi/filesystem.cpp index 3708c15d..e4b3443a 100644 --- a/kernel/kapi/filesystem.cpp +++ b/kernel/kapi/filesystem.cpp @@ -1,5 +1,7 @@ #include +#include +#include #include #include #include @@ -28,10 +30,17 @@ namespace kapi::filesystem auto open(std::string_view path) -> kstd::result { - return kernel::filesystem::vfs::get().open(path).and_then([](auto dentry) { - auto open_file_descriptor = kstd::make_shared(dentry); - return kernel::filesystem::open_file_table::get().add_file(open_file_descriptor); - }); + return kernel::filesystem::vfs::get() + .open(path) + .transform([](auto dentry) -> kstd::shared_ptr { + if (dentry->inode()->is_directory()) + { + return kstd::make_shared(dentry); + } + return kstd::make_shared(dentry); + }) + .and_then( + [](auto file_descriptor) { return kernel::filesystem::open_file_table::get().add_file(file_descriptor); }); } auto close(size_t file_descriptor) -> kstd::result diff --git a/kernel/kernel/filesystem/byte_offset_file_descriptor.cpp b/kernel/kernel/filesystem/byte_offset_file_descriptor.cpp new file mode 100644 index 00000000..6071155c --- /dev/null +++ b/kernel/kernel/filesystem/byte_offset_file_descriptor.cpp @@ -0,0 +1,83 @@ +#include + +#include + +#include + +#include +#include +#include + +#include +#include + +namespace kernel::filesystem +{ + + auto byte_offset_file_descriptor::read(std::span buffer) -> kstd::result + { + if (auto result = get_dentry()->inode()->read(buffer, m_offset); !result) + { + return kstd::failure(result.error()); + } + else + { + auto read_bytes = result.value(); + m_offset += read_bytes; + return read_bytes; + } + } + + auto byte_offset_file_descriptor::write(std::span buffer) -> kstd::result + { + if (auto result = get_dentry()->inode()->write(buffer, m_offset); !result) + { + return kstd::failure(result.error()); + } + else + { + auto written_bytes = result.value(); + m_offset += written_bytes; + return written_bytes; + } + } + + auto byte_offset_file_descriptor::seek(kstd::offset offset, kapi::filesystem::seek_origin origin) + -> kstd::result + { + auto const base = [&] { + switch (origin) + { + case kapi::filesystem::seek_origin::beginning: + { + return kstd::bytes{0}; + } + case kapi::filesystem::seek_origin::current_position: + { + return m_offset; + } + case kapi::filesystem::seek_origin::end: + { + return get_dentry()->inode()->status()->size; + } + default: + { + return kstd::bytes{0}; + } + } + }(); + + auto new_offset = base + offset; + if (new_offset) + { + m_offset = *new_offset; + } + return new_offset; + } + + auto byte_offset_file_descriptor::offset() const noexcept -> kstd::bytes + { + return m_offset; + } + +} // namespace kernel::filesystem \ No newline at end of file diff --git a/kernel/kernel/filesystem/byte_offset_file_descriptor.hpp b/kernel/kernel/filesystem/byte_offset_file_descriptor.hpp new file mode 100644 index 00000000..50eedb67 --- /dev/null +++ b/kernel/kernel/filesystem/byte_offset_file_descriptor.hpp @@ -0,0 +1,41 @@ +#ifndef TEACHOS_KERNEL_FILESYSTEM_BYTE_OFFSET_FILE_DESCRIPTOR_HPP +#define TEACHOS_KERNEL_FILESYSTEM_BYTE_OFFSET_FILE_DESCRIPTOR_HPP + +#include +#include +#include + +#include + +#include +#include +#include + +#include +#include + +namespace kernel::filesystem +{ + //! An open file. + //! + //! This class encapsulates the state of an open file, including a reference to the associated directory and the + //! current byte offset within the file. + struct byte_offset_file_descriptor : open_file_descriptor + { + using open_file_descriptor::open_file_descriptor; + + auto read(std::span buffer) -> kstd::result override; + + auto write(std::span buffer) -> kstd::result override; + + auto seek(kstd::offset offset, kapi::filesystem::seek_origin origin) -> kstd::result override; + + [[nodiscard]] auto offset() const noexcept -> kstd::bytes; + + private: + kstd::bytes m_offset{}; + }; + +} // namespace kernel::filesystem + +#endif \ No newline at end of file diff --git a/kernel/kernel/filesystem/byte_offset_file_descriptor.tests.cpp b/kernel/kernel/filesystem/byte_offset_file_descriptor.tests.cpp new file mode 100644 index 00000000..a17ce686 --- /dev/null +++ b/kernel/kernel/filesystem/byte_offset_file_descriptor.tests.cpp @@ -0,0 +1,219 @@ +#include + +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +#include +#include +#include +#include +#include + +#include + +#include +#include +#include +#include +#include + +using namespace kstd::units_literals; + +// NOLINTBEGIN(readability-magic-numbers) + +SCENARIO("Open file descriptor construction", "[filesystem][byte_offset_file_descriptor]") +{ + GIVEN("a dentry and an open file descriptor for that dentry") + { + auto inode = kstd::make_shared(); + auto dentry = kstd::make_shared(nullptr, inode, "test_dentry"); + auto file_descriptor = kernel::filesystem::byte_offset_file_descriptor{dentry}; + + THEN("the initial offset is zero") + { + REQUIRE(file_descriptor.offset() == 0_B); + } + } +} + +SCENARIO("Open file descriptor read/write offset management", "[filesystem][byte_offset_file_descriptor]") +{ + GIVEN("a dentry that tracks read/write calls and an open file descriptor for that dentry") + { + auto inode = kstd::make_shared(); + auto dentry = kstd::make_shared(nullptr, inode, "test_dentry"); + auto file_descriptor = kernel::filesystem::byte_offset_file_descriptor{dentry}; + + THEN("the offset is updated correctly after reads") + { + auto buffer = std::vector{100}; + + REQUIRE(file_descriptor.read(buffer) == 100_B); + REQUIRE(file_descriptor.offset() == 100_B); + REQUIRE(file_descriptor.read(std::span{buffer}.subspan(0, 50)) == 50_B); + REQUIRE(file_descriptor.offset() == 150_B); + } + + THEN("the offset is updated correctly after writes") + { + auto buffer = std::vector{200}; + + REQUIRE(file_descriptor.write(buffer) == 200_B); + REQUIRE(file_descriptor.offset() == 200_B); + REQUIRE(file_descriptor.write(std::span{buffer}.subspan(0, 25)) == 25_B); + REQUIRE(file_descriptor.offset() == 225_B); + } + + THEN("reads and writes both update the same offset") + { + auto buffer = std::vector{20}; + + REQUIRE(file_descriptor.read(std::span{buffer}.subspan(0, 10)) == 10_B); + REQUIRE(file_descriptor.offset() == 10_B); + REQUIRE(file_descriptor.write(std::span{buffer}.subspan(0, 20)) == 20_B); + REQUIRE(file_descriptor.offset() == 30_B); + REQUIRE(file_descriptor.read(std::span{buffer}.subspan(0, 5)) == 5_B); + REQUIRE(file_descriptor.offset() == 35_B); + REQUIRE(file_descriptor.write(std::span{buffer}.subspan(0, 15)) == 15_B); + REQUIRE(file_descriptor.offset() == 50_B); + } + } +} + +SCENARIO_METHOD(kernel::tests::filesystem::storage_boot_module_vfs_fixture, "Open file descriptor read with real image", + "[filesystem][byte_offset_file_descriptor][img]") +{ + auto const image_path = std::filesystem::path{KERNEL_TEST_ASSETS_DIR} / "ext2_1KB_fs.img"; + + 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})); + + auto & vfs = kernel::filesystem::vfs::get(); + auto dentry = vfs.open("/information/info_1.txt"); + REQUIRE(dentry); + auto ofd = kstd::make_shared(dentry.value()); + + THEN("the file can be read and the offset is updated") + { + kstd::vector buffer(32); + auto bytes_read = ofd->read(buffer); + REQUIRE(bytes_read == 7_B); + REQUIRE(ofd->offset() == 7_B); + + std::string_view buffer_as_str{reinterpret_cast(buffer.data()), bytes_read->value}; + REQUIRE(buffer_as_str == "info_1\n"); + } + + THEN("the file can be read multiple times and the offset is updated") + { + kstd::vector buffer(4); + auto bytes_read_1 = ofd->read(std::span{buffer}.first(buffer.size() / 2)); + REQUIRE(bytes_read_1 == kstd::bytes{buffer.size() / 2}); + REQUIRE(ofd->offset() == kstd::bytes{buffer.size() / 2}); + + auto bytes_read_2 = ofd->read(std::span{buffer}.last(buffer.size() / 2)); + REQUIRE(bytes_read_2 == kstd::bytes{buffer.size() / 2}); + REQUIRE(ofd->offset() == kstd::bytes{buffer.size()}); + + std::string_view buffer_as_str{reinterpret_cast(buffer.data()), + bytes_read_1->value + bytes_read_2->value}; + REQUIRE(buffer_as_str == "info"); + } + + THEN("the file can be written to and the offset is updated") + { + auto write_buffer = kstd::vector(12, std::byte{0xAA}); + auto const bytes_written = ofd->write(write_buffer); + REQUIRE(bytes_written == 12_B); + REQUIRE(ofd->offset() == 12_B); + } + + THEN("the file can be written to multiple times and the offset is updated") + { + auto write_buffer = kstd::vector(8, std::byte{0xAA}); + auto const bytes_written_1 = ofd->write(std::span{write_buffer}.first(write_buffer.size() / 2)); + REQUIRE(bytes_written_1 == kstd::bytes{write_buffer.size() / 2}); + REQUIRE(ofd->offset() == kstd::bytes{write_buffer.size() / 2}); + + auto const bytes_written_2 = ofd->write(std::span{write_buffer}.last(write_buffer.size() / 2)); + REQUIRE(bytes_written_2 == kstd::bytes{write_buffer.size() / 2}); + REQUIRE(ofd->offset() == kstd::bytes{write_buffer.size()}); + } + } +} + +SCENARIO_METHOD(kernel::tests::filesystem::storage_boot_module_fixture, "Open file descriptor handles device removal", + "[filesystem][byte_offset_file_descriptor]") +{ + GIVEN("A file descriptor open on a bound, published devices") + { + setup_modules(1); + + auto device = kernel::devices::storage::determine_boot_device(); + REQUIRE(device); + + auto weak_reference = kstd::weak_ptr{device}; + + auto device_inode = kstd::make_shared(device); + auto dentry = kstd::make_shared(nullptr, device_inode, "ram0"); + auto fd = kstd::make_shared(dentry); + + device.reset(); + REQUIRE_FALSE(weak_reference.expired()); + + WHEN("the device is removed while the descriptor is still open") + { + auto locked = weak_reference.lock(); + REQUIRE(locked); + REQUIRE(kapi::devices::remove_device(*locked)); + locked.reset(); + + THEN("the device is kept alive through the file descriptor") + { + REQUIRE_FALSE(weak_reference.expired()); + } + + THEN("reading through the file descriptor fails with 'no such device'") + { + auto buffer = kstd::vector(10); + auto bytes_read = fd->read(buffer); + + REQUIRE(bytes_read.error() == kstd::errc::no_such_device); + } + + THEN("writing through the file descriptor fails with 'no such device'") + { + auto buffer = kstd::vector(10); + auto bytes_written = fd->write(buffer); + + REQUIRE(bytes_written.error() == kstd::errc::no_such_device); + } + + THEN("resetting the chain destroys the device") + { + REQUIRE_FALSE(weak_reference.expired()); + + fd.reset(); + REQUIRE_FALSE(weak_reference.expired()); + + dentry.reset(); + REQUIRE_FALSE(weak_reference.expired()); + + device_inode.reset(); + REQUIRE(weak_reference.expired()); + } + } + } +} + +// NOLINTEND(readability-magic-numbers) \ No newline at end of file diff --git a/kernel/kernel/filesystem/directory_file_descriptor.cpp b/kernel/kernel/filesystem/directory_file_descriptor.cpp new file mode 100644 index 00000000..8c73ae86 --- /dev/null +++ b/kernel/kernel/filesystem/directory_file_descriptor.cpp @@ -0,0 +1,34 @@ +#include + +#include +#include +#include + +#include +#include + +#include +#include + +namespace kernel::filesystem +{ + + auto directory_file_descriptor::read_directory(std::span entries) + -> kstd::result + { + auto result = get_dentry()->inode()->read_directory(m_position, entries); + if (!result) + { + return kstd::failure(result.error()); + } + + m_position = result->second; + return result->first; + } + + auto directory_file_descriptor::position() const noexcept -> directory_listing_cursor + { + return m_position; + } + +} // namespace kernel::filesystem \ No newline at end of file diff --git a/kernel/kernel/filesystem/directory_file_descriptor.hpp b/kernel/kernel/filesystem/directory_file_descriptor.hpp new file mode 100644 index 00000000..075937ca --- /dev/null +++ b/kernel/kernel/filesystem/directory_file_descriptor.hpp @@ -0,0 +1,35 @@ +#ifndef TEACHOS_KERNEL_FILESYSTEM_DIRECTORY_FILE_DESCRIPTOR_HPP +#define TEACHOS_KERNEL_FILESYSTEM_DIRECTORY_FILE_DESCRIPTOR_HPP + +#include +#include +#include +#include + +#include + +#include +#include +#include + +#include +#include + +namespace kernel::filesystem +{ + + struct directory_file_descriptor : open_file_descriptor + { + using open_file_descriptor::open_file_descriptor; + + auto read_directory(std::span entries) -> kstd::result override; + + [[nodiscard]] auto position() const noexcept -> directory_listing_cursor; + + private: + directory_listing_cursor m_position{}; + }; + +} // namespace kernel::filesystem + +#endif \ No newline at end of file diff --git a/kernel/kernel/filesystem/directory_file_descriptor.tests.cpp b/kernel/kernel/filesystem/directory_file_descriptor.tests.cpp new file mode 100644 index 00000000..ac60aad9 --- /dev/null +++ b/kernel/kernel/filesystem/directory_file_descriptor.tests.cpp @@ -0,0 +1,81 @@ +#include + +#include +#include +#include +#include +#include +#include + +#include + +#include +#include + +#include + +#include +#include +#include +#include +#include + +SCENARIO_METHOD(kernel::tests::filesystem::storage_boot_module_vfs_fixture, + "directory_file_descriptor owns its cursor across calls, not the caller", + "[filesystem][directory_file_descriptor][img]") +{ + auto const image_path = std::filesystem::path{KERNEL_TEST_ASSETS_DIR} / "ext2_1KB_fs.img"; + + GIVEN("a directory_file_descriptor open on a real directory") + { + REQUIRE(std::filesystem::exists(image_path)); + REQUIRE_NOTHROW(setup_modules_from_img_and_init_vfs({"test_img_module"}, {image_path})); + + auto & vfs = kernel::filesystem::vfs::get(); + auto dentry = vfs.open("/information"); + REQUIRE(dentry); + + auto fd = kernel::filesystem::directory_file_descriptor{dentry.value()}; + + THEN("position starts at a default-constructed cursor") + { + REQUIRE(fd.position() == kernel::filesystem::directory_listing_cursor{}); + } + + WHEN("read_directory is called one entry at a time, with no cursor passed by the caller") + { + auto seen = std::vector{}; + + for (auto guard = 0; guard < 16; ++guard) + { + auto one = kstd::vector(1); + auto result = fd.read_directory(one); + REQUIRE(result); + if (*result == 0) + { + break; + } + seen.push_back(std::string{one[0].name}); + } + + THEN("every call advanced position on its own, and the known entries all appear exactly once") + { + REQUIRE(fd.position() != kernel::filesystem::directory_listing_cursor{}); + REQUIRE(std::ranges::find(seen, ".") != seen.end()); + REQUIRE(std::ranges::find(seen, "info_1.txt") != seen.end()); + + AND_THEN("a repeated call after exhaustion returns zero without changing position again") + { + auto const position_at_exhaustion = fd.position(); + + auto one = kstd::vector(1); + auto result = fd.read_directory(one); + + REQUIRE(result); + REQUIRE(*result == 0); + REQUIRE(fd.position() == position_at_exhaustion); + } + } + } + } +} \ No newline at end of file diff --git a/kernel/kernel/filesystem/open_file_descriptor.cpp b/kernel/kernel/filesystem/open_file_descriptor.cpp index 64c47461..d18ae141 100644 --- a/kernel/kernel/filesystem/open_file_descriptor.cpp +++ b/kernel/kernel/filesystem/open_file_descriptor.cpp @@ -1,6 +1,8 @@ #include #include +#include +#include #include @@ -16,7 +18,6 @@ namespace kernel::filesystem { open_file_descriptor::open_file_descriptor(kstd::shared_ptr const & dentry) : m_dentry(dentry) - , m_offset(0) { if (!dentry) { @@ -24,70 +25,24 @@ namespace kernel::filesystem } } - auto open_file_descriptor::read(std::span buffer) -> kstd::result + auto open_file_descriptor::read(std::span) -> kstd::result { - if (auto result = m_dentry->inode()->read(buffer, m_offset); !result) - { - return kstd::failure(result.error()); - } - else - { - auto read_bytes = result.value(); - m_offset += read_bytes; - return read_bytes; - } + return kstd::failure(vfs_errc::is_a_directory); } - auto open_file_descriptor::write(std::span buffer) -> kstd::result + auto open_file_descriptor::write(std::span) -> kstd::result { - if (auto result = m_dentry->inode()->write(buffer, m_offset); !result) - { - return kstd::failure(result.error()); - } - else - { - auto written_bytes = result.value(); - m_offset += written_bytes; - return written_bytes; - } + return kstd::failure(vfs_errc::is_a_directory); } - auto open_file_descriptor::seek(kstd::offset offset, kapi::filesystem::seek_origin origin) - -> kstd::result + auto open_file_descriptor::seek(kstd::offset, kapi::filesystem::seek_origin) -> kstd::result { - auto const base = [&] { - switch (origin) - { - case kapi::filesystem::seek_origin::beginning: - { - return kstd::bytes{0}; - } - case kapi::filesystem::seek_origin::current_position: - { - return m_offset; - } - case kapi::filesystem::seek_origin::end: - { - return m_dentry->inode()->status()->size; - } - default: - { - return kstd::bytes{0}; - } - } - }(); - - auto new_offset = base + offset; - if (new_offset) - { - m_offset = *new_offset; - } - return new_offset; + return kstd::failure(vfs_errc::is_a_directory); } - auto open_file_descriptor::offset() const -> kstd::bytes + auto open_file_descriptor::read_directory(std::span) -> kstd::result { - return m_offset; + return kstd::failure(vfs_errc::not_a_directory); } auto open_file_descriptor::get_dentry() const -> kstd::shared_ptr const & diff --git a/kernel/kernel/filesystem/open_file_descriptor.hpp b/kernel/kernel/filesystem/open_file_descriptor.hpp index 735ea7a2..3f3722b4 100644 --- a/kernel/kernel/filesystem/open_file_descriptor.hpp +++ b/kernel/kernel/filesystem/open_file_descriptor.hpp @@ -2,6 +2,7 @@ #define TEACHOS_KERNEL_FILESYSTEM_OPEN_FILE_DESCRIPTOR_HPP #include +#include #include @@ -25,29 +26,33 @@ namespace kernel::filesystem //! @param dentry The directory entry to associate with the open file descriptor. explicit open_file_descriptor(kstd::shared_ptr const & dentry); + //! Virtual destructor to enable safe destruction through base pointers. + virtual ~open_file_descriptor() = default; + //! Read data from the open file descriptor into a buffer. //! //! @param buffer The buffer to read data into. //! @return The number of bytes read on success, an error otherwise. - auto read(std::span buffer) -> kstd::result; + virtual auto read(std::span buffer) -> kstd::result; //! Write data to the open file descriptor from a buffer. //! //! @param buffer The buffer to write data from. //! @return The number of bytes written on success, an error otherwise. - auto write(std::span buffer) -> kstd::result; + virtual auto write(std::span buffer) -> kstd::result; //! Move the read/write offset of the file. //! //! @param offset The offset to apply relative to the given origin. //! @param origin The origin of the offset. //! @return the new offset on success, an error otherwise. - auto seek(kstd::offset offset, kapi::filesystem::seek_origin origin) -> kstd::result; + virtual auto seek(kstd::offset offset, kapi::filesystem::seek_origin origin) -> kstd::result; - //! Get the current file offset for this open file descriptor. + //! Read directory entries from the file. //! - //! @return The current file offset in bytes. - [[nodiscard]] auto offset() const -> kstd::bytes; + //! @param entries A buffer to read the entries into. + //! @return The number of read entries on success, an error otherwise. + virtual auto read_directory(std::span entries) -> kstd::result; //! Get a reference to the directory entry associated with this open file descriptor. //! @@ -56,7 +61,6 @@ namespace kernel::filesystem private: kstd::shared_ptr m_dentry; - kstd::bytes m_offset; }; } // namespace kernel::filesystem diff --git a/kernel/kernel/filesystem/open_file_descriptor.tests.cpp b/kernel/kernel/filesystem/open_file_descriptor.tests.cpp index a2e30c16..2fbffbf8 100644 --- a/kernel/kernel/filesystem/open_file_descriptor.tests.cpp +++ b/kernel/kernel/filesystem/open_file_descriptor.tests.cpp @@ -1,219 +1,110 @@ #include -#include +#include #include -#include -#include -#include +#include +#include +#include #include -#include -#include -#include +#include #include -#include -#include #include #include #include #include -#include #include -#include -#include using namespace kstd::units_literals; -// NOLINTBEGIN(readability-magic-numbers) - -SCENARIO("Open file descriptor construction", "[filesystem][open_file_descriptor]") +SCENARIO("open_file_descriptor's base defaults fail every operation, unconditionally", + "[filesystem][open_file_descriptor]") { - GIVEN("a dentry and an open file descriptor for that dentry") + GIVEN("a base open_file_descriptor constructed directly, not through a derived type") { auto inode = kstd::make_shared(); auto dentry = kstd::make_shared(nullptr, inode, "test_dentry"); - auto file_descriptor = kernel::filesystem::open_file_descriptor{dentry}; + auto fd = kernel::filesystem::open_file_descriptor{dentry}; - THEN("the initial offset is zero") + THEN("read fails as 'this is a directory', not silently succeeding with zero bytes") { - REQUIRE(file_descriptor.offset() == 0_B); - } - } -} + auto buffer = kstd::vector(4); + auto result = fd.read(buffer); -SCENARIO("Open file descriptor read/write offset management", "[filesystem][open_file_descriptor]") -{ - GIVEN("a dentry that tracks read/write calls and an open file descriptor for that dentry") - { - auto inode = kstd::make_shared(); - auto dentry = kstd::make_shared(nullptr, inode, "test_dentry"); - auto file_descriptor = kernel::filesystem::open_file_descriptor{dentry}; + REQUIRE_FALSE(result); + REQUIRE(result.error() == kernel::filesystem::vfs_errc::is_a_directory); + } - THEN("the offset is updated correctly after reads") + THEN("write fails the same way") { - auto buffer = std::vector{100}; + auto buffer = kstd::vector{std::byte{'x'}}; + auto result = fd.write(buffer); - REQUIRE(file_descriptor.read(buffer) == 100_B); - REQUIRE(file_descriptor.offset() == 100_B); - REQUIRE(file_descriptor.read(std::span{buffer}.subspan(0, 50)) == 50_B); - REQUIRE(file_descriptor.offset() == 150_B); + REQUIRE_FALSE(result); + REQUIRE(result.error() == kernel::filesystem::vfs_errc::is_a_directory); } - THEN("the offset is updated correctly after writes") + THEN("seek fails the same way") { - auto buffer = std::vector{200}; + auto result = fd.seek(kstd::offset{0}, kapi::filesystem::seek_origin::beginning); - REQUIRE(file_descriptor.write(buffer) == 200_B); - REQUIRE(file_descriptor.offset() == 200_B); - REQUIRE(file_descriptor.write(std::span{buffer}.subspan(0, 25)) == 25_B); - REQUIRE(file_descriptor.offset() == 225_B); + REQUIRE_FALSE(result); + REQUIRE(result.error() == kernel::filesystem::vfs_errc::is_a_directory); } - THEN("reads and writes both update the same offset") + THEN("read_directory fails as 'this is not a directory' — the opposite error, and the only one of " + "the four that's correct for it") { - auto buffer = std::vector{20}; - - REQUIRE(file_descriptor.read(std::span{buffer}.subspan(0, 10)) == 10_B); - REQUIRE(file_descriptor.offset() == 10_B); - REQUIRE(file_descriptor.write(std::span{buffer}.subspan(0, 20)) == 20_B); - REQUIRE(file_descriptor.offset() == 30_B); - REQUIRE(file_descriptor.read(std::span{buffer}.subspan(0, 5)) == 5_B); - REQUIRE(file_descriptor.offset() == 35_B); - REQUIRE(file_descriptor.write(std::span{buffer}.subspan(0, 15)) == 15_B); - REQUIRE(file_descriptor.offset() == 50_B); + auto entries = kstd::vector(1); + auto result = fd.read_directory(entries); + + REQUIRE_FALSE(result); + REQUIRE(result.error() == kernel::filesystem::vfs_errc::not_a_directory); } } } -SCENARIO_METHOD(kernel::tests::filesystem::storage_boot_module_vfs_fixture, "Open file descriptor read with real image", - "[filesystem][open_file_descriptor][img]") +SCENARIO("Cross-type calls through open_file_descriptor fall through to the correct base default, not " + "the other subclass's behavior", + "[filesystem][open_file_descriptor]") { - auto const image_path = std::filesystem::path{KERNEL_TEST_ASSETS_DIR} / "ext2_1KB_fs.img"; - - GIVEN("an open file descriptor for a file in a real image") + GIVEN("a byte_offset_file_descriptor, accessed through a base pointer") { - REQUIRE(std::filesystem::exists(image_path)); - REQUIRE_NOTHROW(setup_modules_from_img_and_init_vfs({"test_img_module"}, {image_path})); - - auto & vfs = kernel::filesystem::vfs::get(); - auto dentry = vfs.open("/information/info_1.txt"); - REQUIRE(dentry); - auto ofd = kstd::make_shared(dentry.value()); - - THEN("the file can be read and the offset is updated") - { - kstd::vector buffer(32); - auto bytes_read = ofd->read(buffer); - REQUIRE(bytes_read == 7_B); - REQUIRE(ofd->offset() == 7_B); - - std::string_view buffer_as_str{reinterpret_cast(buffer.data()), bytes_read->value}; - REQUIRE(buffer_as_str == "info_1\n"); - } - - THEN("the file can be read multiple times and the offset is updated") - { - kstd::vector buffer(4); - auto bytes_read_1 = ofd->read(std::span{buffer}.first(buffer.size() / 2)); - REQUIRE(bytes_read_1 == kstd::bytes{buffer.size() / 2}); - REQUIRE(ofd->offset() == kstd::bytes{buffer.size() / 2}); - - auto bytes_read_2 = ofd->read(std::span{buffer}.last(buffer.size() / 2)); - REQUIRE(bytes_read_2 == kstd::bytes{buffer.size() / 2}); - REQUIRE(ofd->offset() == kstd::bytes{buffer.size()}); - - std::string_view buffer_as_str{reinterpret_cast(buffer.data()), - bytes_read_1->value + bytes_read_2->value}; - REQUIRE(buffer_as_str == "info"); - } + auto inode = kstd::make_shared(); + auto dentry = kstd::make_shared(nullptr, inode, "test_dentry"); + kstd::shared_ptr fd = + kstd::make_shared(dentry); - THEN("the file can be written to and the offset is updated") + THEN("read_directory falls through to the base — byte_offset_file_descriptor never overrode it") { - auto write_buffer = kstd::vector(12, std::byte{0xAA}); - auto const bytes_written = ofd->write(write_buffer); - REQUIRE(bytes_written == 12_B); - REQUIRE(ofd->offset() == 12_B); - } + auto entries = kstd::vector(1); + auto result = fd->read_directory(entries); - THEN("the file can be written to multiple times and the offset is updated") - { - auto write_buffer = kstd::vector(8, std::byte{0xAA}); - auto const bytes_written_1 = ofd->write(std::span{write_buffer}.first(write_buffer.size() / 2)); - REQUIRE(bytes_written_1 == kstd::bytes{write_buffer.size() / 2}); - REQUIRE(ofd->offset() == kstd::bytes{write_buffer.size() / 2}); - - auto const bytes_written_2 = ofd->write(std::span{write_buffer}.last(write_buffer.size() / 2)); - REQUIRE(bytes_written_2 == kstd::bytes{write_buffer.size() / 2}); - REQUIRE(ofd->offset() == kstd::bytes{write_buffer.size()}); + REQUIRE_FALSE(result); + REQUIRE(result.error() == kernel::filesystem::vfs_errc::not_a_directory); } } -} -SCENARIO_METHOD(kernel::tests::filesystem::storage_boot_module_fixture, "Open file descriptor handles device removal", - "[filesystem][open_file_descriptor]") -{ - GIVEN("A file descriptor open on a bound, published devices") + GIVEN("a directory_file_descriptor, accessed through a base pointer") { - setup_modules(1); - - auto device = kernel::devices::storage::determine_boot_device(); - REQUIRE(device); - - auto weak_reference = kstd::weak_ptr{device}; - - auto device_inode = kstd::make_shared(device); - auto dentry = kstd::make_shared(nullptr, device_inode, "ram0"); - auto fd = kstd::make_shared(dentry); - - device.reset(); - REQUIRE_FALSE(weak_reference.expired()); + auto inode = kstd::make_shared(); + auto dentry = kstd::make_shared(nullptr, inode, "test_dentry"); + kstd::shared_ptr fd = + kstd::make_shared(dentry); - WHEN("the device is removed while the descriptor is still open") + THEN("read/write/seek all fall through to the base — directory_file_descriptor never overrode any " + "of them") { - auto locked = weak_reference.lock(); - REQUIRE(locked); - REQUIRE(kapi::devices::remove_device(*locked)); - locked.reset(); - - THEN("the device is kept alive through the file descriptor") - { - REQUIRE_FALSE(weak_reference.expired()); - } - - THEN("reading through the file descriptor fails with 'no such device'") - { - auto buffer = kstd::vector(10); - auto bytes_read = fd->read(buffer); - - REQUIRE(bytes_read.error() == kstd::errc::no_such_device); - } - - THEN("writing through the file descriptor fails with 'no such device'") - { - auto buffer = kstd::vector(10); - auto bytes_written = fd->write(buffer); + auto buffer = kstd::vector(4); + REQUIRE_FALSE(fd->read(buffer)); - REQUIRE(bytes_written.error() == kstd::errc::no_such_device); - } + auto write_buffer = kstd::vector{std::byte{'x'}}; + REQUIRE_FALSE(fd->write(write_buffer)); - THEN("resetting the chain destroys the device") - { - REQUIRE_FALSE(weak_reference.expired()); - - fd.reset(); - REQUIRE_FALSE(weak_reference.expired()); - - dentry.reset(); - REQUIRE_FALSE(weak_reference.expired()); - - device_inode.reset(); - REQUIRE(weak_reference.expired()); - } + REQUIRE_FALSE(fd->seek(kstd::offset{0}, kapi::filesystem::seek_origin::beginning)); } } -} - -// NOLINTEND(readability-magic-numbers) \ No newline at end of file +} \ No newline at end of file diff --git a/kernel/kernel/filesystem/vfs.tests.cpp b/kernel/kernel/filesystem/vfs.tests.cpp index 5b7ea621..aac384fb 100644 --- a/kernel/kernel/filesystem/vfs.tests.cpp +++ b/kernel/kernel/filesystem/vfs.tests.cpp @@ -1,5 +1,6 @@ #include +#include #include #include #include @@ -400,7 +401,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.value()); + auto sheep_1_ofd = kstd::make_shared(dentry.value()); kstd::vector buffer(7); auto bytes_read = sheep_1_ofd->read(buffer); @@ -426,8 +427,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.value()); - auto goat_1_ofd = kstd::make_shared(goat_1.value()); + auto sheep_1_ofd = kstd::make_shared(sheep_1.value()); + auto goat_1_ofd = kstd::make_shared(goat_1.value()); kstd::vector sheep_buffer(7); auto bytes_read = sheep_1_ofd->read(sheep_buffer); -- cgit v1.2.3