From 40e71b2af97f8827be5d88352414365193145ed3 Mon Sep 17 00:00:00 2001 From: Nathan Clack Date: Thu, 21 Sep 2023 11:03:16 -0700 Subject: [PATCH] File creation behavior (#33) closes #32 Adds a test, and modifies `create_file()` implementations across supported platforms so that they exhibit the following behavior: 1. When creating a file for a path that already exists, creation should succeed, truncating the original file. 2. When creating a file (for writing) that has already been opened for the process, creation should fail, and the original file should be unaffected. - [x] windows - CreateFile api just does what we want. That one was already working. - [x] osx - has a special flag we can pass to `open` that does what we want. - [x] linux - use the `flock` api. According to `man 2 flock` this has been a syscall since kernel 2.0. Potentially racy, but that's probably ok. --- src/acquire-core-platform/linux/platform.c | 19 ++++- src/acquire-core-platform/osx/platform.c | 10 ++- tests/CMakeLists.txt | 2 +- tests/file-create-behavior.cpp | 85 ++++++++++++++++++++++ 4 files changed, 109 insertions(+), 7 deletions(-) create mode 100644 tests/file-create-behavior.cpp diff --git a/src/acquire-core-platform/linux/platform.c b/src/acquire-core-platform/linux/platform.c index eb0f855..946180d 100644 --- a/src/acquire-core-platform/linux/platform.c +++ b/src/acquire-core-platform/linux/platform.c @@ -8,6 +8,7 @@ #include #include #include +#include #define L (aq_logger) #define LOG(...) L(0, __FILE__, __LINE__, __FUNCTION__, __VA_ARGS__) @@ -36,9 +37,17 @@ int file_create(struct file* file, const char* filename, size_t bytesof_filename) { - file->fid = open(filename, O_RDWR | O_CREAT | O_EXCL | O_NONBLOCK, 0666); + file->fid = open(filename, O_RDWR | O_CREAT | O_NONBLOCK, 0666); if (file->fid < 0) CHECK_POSIX(errno); + { + int ret = flock(file->fid, LOCK_EX | LOCK_NB); + if (ret < 0) { + int tmp = errno; + close(file->fid); + CHECK_POSIX(tmp); + } + } return 1; Error: return 0; @@ -47,7 +56,8 @@ file_create(struct file* file, const char* filename, size_t bytesof_filename) void file_close(struct file* file) { - CHECK_POSIX(close(file->fid)); + if (close(file->fid) < 0) + CHECK_POSIX(errno); Error:; } @@ -77,8 +87,11 @@ int file_exists(const char* filename, size_t nbytes) { int ret = access(filename, F_OK); - if (ret < 0) + if (ret < 0) { + if (errno == ENOENT) + return 0; CHECK_POSIX(errno); + } return ret == 0; Error: return 0; diff --git a/src/acquire-core-platform/osx/platform.c b/src/acquire-core-platform/osx/platform.c index bf94fbd..40c6b1b 100644 --- a/src/acquire-core-platform/osx/platform.c +++ b/src/acquire-core-platform/osx/platform.c @@ -44,7 +44,7 @@ int file_create(struct file* file, const char* filename, size_t bytesof_filename) { - file->fid = open(filename, O_RDWR | O_CREAT | O_EXCL | O_NONBLOCK, 0666); + file->fid = open(filename, O_RDWR | O_CREAT | O_EXLOCK | O_NONBLOCK, 0666); if (file->fid < 0) CHECK_POSIX(errno); return 1; @@ -55,7 +55,8 @@ file_create(struct file* file, const char* filename, size_t bytesof_filename) void file_close(struct file* file) { - CHECK_POSIX(close(file->fid)); + if (close(file->fid) < 0) + CHECK_POSIX(errno); Error:; } @@ -85,8 +86,11 @@ int file_exists(const char* filename, size_t nbytes) { int ret = access(filename, F_OK); - if (ret < 0) + if (ret < 0) { + if (errno == ENOENT) + return 0; CHECK_POSIX(errno); + } return ret == 0; Error: return 0; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 3c134fa..92a86bd 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -6,6 +6,7 @@ else() foreach(name unit-tests instance-types + file-create-behavior ) set(tgt "${project}-${name}") add_executable(${tgt} ${name}.cpp) @@ -19,6 +20,5 @@ else() target_compile_definitions(${tgt} PUBLIC TEST="${tgt}") add_test(NAME test-${tgt} COMMAND ${tgt}) set_tests_properties(test-${tgt} PROPERTIES LABELS "anyplatform;acquire-core-libs") - endforeach() endif() diff --git a/tests/file-create-behavior.cpp b/tests/file-create-behavior.cpp new file mode 100644 index 0000000..57de7b7 --- /dev/null +++ b/tests/file-create-behavior.cpp @@ -0,0 +1,85 @@ +//! Test that we can't create the same file for writing within this process +//! using the file_create() platform api. +#include "platform.h" +#include "logger.h" + +#include +#include + +/// Helper for passing size static strings as function args. +/// For a function: `f(char*,size_t)` use `f(SIZED("hello"))`. +/// Expands to `f("hello",6)`. +#define SIZED(str) str, sizeof(str) + +#define L (aq_logger) +#define LOG(...) L(0, __FILE__, __LINE__, __FUNCTION__, __VA_ARGS__) +#define ERR(...) L(1, __FILE__, __LINE__, __FUNCTION__, __VA_ARGS__) +#define EXPECT(e, ...) \ + do { \ + if (!(e)) { \ + char buf[1 << 8] = { 0 }; \ + ERR(__VA_ARGS__); \ + snprintf(buf, sizeof(buf) - 1, __VA_ARGS__); \ + throw std::runtime_error(buf); \ + } \ + } while (0) +#define CHECK(e) EXPECT(e, "Expression evaluated as false: %s", #e) + +void +reporter(int is_error, + const char* file, + int line, + const char* function, + const char* msg) +{ + fprintf(is_error ? stderr : stdout, + "%s%s(%d) - %s: %s\n", + is_error ? "ERROR " : "", + file, + line, + function, + msg); +} + +int +main(int argc, char** argv) +{ + logger_set_reporter(reporter); + + const char filename[] = "does-not-exist"; + + remove(filename); + + try { + struct file file; + + EXPECT(0 == file_exists(SIZED(filename)), + "The file \"%s\" must not already be present at the start of " + "this test.", + filename); + + EXPECT(file_create(&file, SIZED(filename)), + "Expected the first creation of \"%s\" to succeed.", + filename); + file_close(&file); + + EXPECT( + file_create(&file, SIZED(filename)), + "Expected creation of \"%s\" to succeed. It was previously created, " + "then closed, so it exists on the file system", + filename); + EXPECT( + !file_create(&file, SIZED(filename)), + "Expected creation of \"%s\" to fail. It hasn't been closed yet.", + filename); + + remove(filename); + return 0; + } catch (const std::exception& e) { + ERR("%s", e.what()); + } catch (...) { + ERR("Unknown exception"); + } + remove(filename); + return 1; +} \ No newline at end of file