check whether the target already exists when creating an inode

5 files changed, 33 insertions, 25 deletions

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>{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<std::byte>(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<char *>(read_buffer.data()), static_cast<size_t>(bytes_read)};
+      std::string_view buffer_as_str{reinterpret_cast<char *>(read_buffer.data()), static_cast<size_t>(*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())