From 1b17e5de9d5a949dc4da6b312af19b06c96e0f2f Mon Sep 17 00:00:00 2001 From: Lukas Oesch Date: Tue, 9 Jun 2026 22:58:49 +0200 Subject: check whether the target already exists when creating an inode --- kernel/kapi/filesystem.tests.cpp | 40 +++++++++++----------- .../kernel/filesystem/devfs/filesystem.tests.cpp | 4 +-- kernel/kernel/filesystem/error.hpp | 5 +++ .../kernel/filesystem/rootfs/filesystem.tests.cpp | 4 +-- kernel/kernel/filesystem/vfs.cpp | 5 ++- 5 files changed, 33 insertions(+), 25 deletions(-) (limited to 'kernel') diff --git a/kernel/kapi/filesystem.tests.cpp b/kernel/kapi/filesystem.tests.cpp index 595d73e8..f2a6a988 100644 --- a/kernel/kapi/filesystem.tests.cpp +++ b/kernel/kapi/filesystem.tests.cpp @@ -216,28 +216,28 @@ SCENARIO_METHOD(kernel::tests::filesystem::storage_boot_module_vfs_fixture, "Kap { auto new_file_path = "/information/new_information.txt"; - REQUIRE(kapi::filesystem::open(new_file_path) < 0); + REQUIRE(!kapi::filesystem::open(new_file_path)); - REQUIRE(kapi::filesystem::create(new_file_path) == 0); + REQUIRE(kapi::filesystem::create(new_file_path)); auto fd = kapi::filesystem::open(new_file_path); - REQUIRE(fd >= 0); + REQUIRE(fd); auto write_buffer = std::vector{std::byte{'T'}, std::byte{'e'}, std::byte{'s'}, std::byte{'t'}}; - kapi::filesystem::write(fd, write_buffer.data(), write_buffer.size()); + CHECK(kapi::filesystem::write(*fd, write_buffer.data(), write_buffer.size())); - REQUIRE(kapi::filesystem::close(fd) == 0); + REQUIRE(kapi::filesystem::close(*fd)); fd = kapi::filesystem::open(new_file_path); - REQUIRE(fd >= 0); + REQUIRE(fd); auto read_buffer = std::vector(4); - auto bytes_read = kapi::filesystem::read(fd, read_buffer.data(), read_buffer.size()); + auto bytes_read = kapi::filesystem::read(*fd, read_buffer.data(), read_buffer.size()); - std::string_view buffer_as_str{reinterpret_cast(read_buffer.data()), static_cast(bytes_read)}; + std::string_view buffer_as_str{reinterpret_cast(read_buffer.data()), static_cast(*bytes_read)}; REQUIRE(buffer_as_str == "Test"); - REQUIRE(kapi::filesystem::close(fd) == 0); + REQUIRE(kapi::filesystem::close(*fd)); } THEN("a new directory can be created and a file within can be created, opened, and closed again") @@ -245,29 +245,29 @@ SCENARIO_METHOD(kernel::tests::filesystem::storage_boot_module_vfs_fixture, "Kap auto new_directory_path = "/information/new_directory"; auto new_file_path = "/information/new_directory/new_information.txt"; - REQUIRE(kapi::filesystem::open(new_file_path) < 0); + REQUIRE(!kapi::filesystem::open(new_file_path)); - REQUIRE(kapi::filesystem::mkdir(new_directory_path) == 0); - REQUIRE(kapi::filesystem::create(new_file_path) == 0); + REQUIRE(kapi::filesystem::mkdir(new_directory_path)); + REQUIRE(kapi::filesystem::create(new_file_path)); auto fd = kapi::filesystem::open(new_file_path); - REQUIRE(fd >= 0); - REQUIRE(kapi::filesystem::close(fd) == 0); + REQUIRE(fd); + REQUIRE(kapi::filesystem::close(*fd)); } THEN("a file can be created in a mounted filesystem, opened, and closed again") { - kapi::filesystem::mount("/dev/ram16", "/information"); + CHECK(kapi::filesystem::mount("/dev/ram16", "/information")); auto new_file_path = "/information/monkey_house/monkey_4.txt"; - REQUIRE(kapi::filesystem::open(new_file_path) < 0); - REQUIRE(kapi::filesystem::create(new_file_path) == 0); + REQUIRE(!kapi::filesystem::open(new_file_path)); + REQUIRE(kapi::filesystem::create(new_file_path)); auto fd = kapi::filesystem::open(new_file_path); - REQUIRE(fd >= 0); - REQUIRE(kapi::filesystem::close(fd) == 0); + REQUIRE(fd); + REQUIRE(kapi::filesystem::close(*fd)); - REQUIRE(kapi::filesystem::umount("/information") == 0); + REQUIRE(kapi::filesystem::umount("/information")); } } } diff --git a/kernel/kernel/filesystem/devfs/filesystem.tests.cpp b/kernel/kernel/filesystem/devfs/filesystem.tests.cpp index bbfddee3..c1aad37c 100644 --- a/kernel/kernel/filesystem/devfs/filesystem.tests.cpp +++ b/kernel/kernel/filesystem/devfs/filesystem.tests.cpp @@ -54,10 +54,10 @@ SCENARIO_METHOD(kernel::tests::filesystem::storage_boot_module_fixture, REQUIRE(!result); } - THEN("create_inode always returns null") + THEN("create_inode always returns an error") { auto result = fs.create_inode(fs.root_inode(), "new_device", kernel::filesystem::vfs_types::inode_type::device); - REQUIRE(result == nullptr); + REQUIRE(!result); } } diff --git a/kernel/kernel/filesystem/error.hpp b/kernel/kernel/filesystem/error.hpp index 82ba33b3..584bf261 100644 --- a/kernel/kernel/filesystem/error.hpp +++ b/kernel/kernel/filesystem/error.hpp @@ -25,6 +25,7 @@ namespace kernel::filesystem not_a_directory, is_a_directory, read_only_file_system, + file_exists, }; namespace detail @@ -68,6 +69,8 @@ namespace kernel::filesystem return "is a directory"; case vfs_errc::read_only_file_system: return "readonly filesystem"; + case vfs_errc::file_exists: + return "file already exists"; default: return "unknown VFS error"; } @@ -100,6 +103,8 @@ namespace kernel::filesystem return make_error_condition(kstd::errc::is_a_directory); case vfs_errc::read_only_file_system: return make_error_condition(kstd::errc::read_only_file_system); + case vfs_errc::file_exists: + return make_error_condition(kstd::errc::file_exists); default: return kstd::error_condition{value, *this}; } diff --git a/kernel/kernel/filesystem/rootfs/filesystem.tests.cpp b/kernel/kernel/filesystem/rootfs/filesystem.tests.cpp index ecd20707..95fa57db 100644 --- a/kernel/kernel/filesystem/rootfs/filesystem.tests.cpp +++ b/kernel/kernel/filesystem/rootfs/filesystem.tests.cpp @@ -36,11 +36,11 @@ SCENARIO("Rootfs filesystem mount and lookup", "[filesystem][rootfs][filesystem] REQUIRE(!result); } - THEN("create_inode always returns null") + THEN("create_inode always returns an error") { auto result = fs.create_inode(fs.root_inode(), "new_device", kernel::filesystem::vfs_types::inode_type::directory); - REQUIRE(result == nullptr); + REQUIRE(!result); } } } diff --git a/kernel/kernel/filesystem/vfs.cpp b/kernel/kernel/filesystem/vfs.cpp index 4691dac3..9a78b6f8 100644 --- a/kernel/kernel/filesystem/vfs.cpp +++ b/kernel/kernel/filesystem/vfs.cpp @@ -189,7 +189,10 @@ namespace kernel::filesystem return kstd::failure(vfs_errc::invalid_path); } - // TODO check whether the target already exists + if (resolve_path(path)) + { + return kstd::failure(vfs_errc::file_exists); + } auto [parent_path, name] = path::split_into_path_and_filename(path); if (name.empty()) -- cgit v1.2.3