Skip to content
This repository has been archived by the owner on Sep 4, 2024. It is now read-only.

Commit

Permalink
File creation behavior (#33)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
nclack authored Sep 21, 2023
1 parent a82da24 commit 40e71b2
Show file tree
Hide file tree
Showing 4 changed files with 109 additions and 7 deletions.
19 changes: 16 additions & 3 deletions src/acquire-core-platform/linux/platform.c
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <fcntl.h>
#include <unistd.h>
#include <dlfcn.h>
#include <sys/file.h>

#define L (aq_logger)
#define LOG(...) L(0, __FILE__, __LINE__, __FUNCTION__, __VA_ARGS__)
Expand Down Expand Up @@ -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;
Expand All @@ -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:;
}

Expand Down Expand Up @@ -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;
Expand Down
10 changes: 7 additions & 3 deletions src/acquire-core-platform/osx/platform.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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:;
}

Expand Down Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ else()
foreach(name
unit-tests
instance-types
file-create-behavior
)
set(tgt "${project}-${name}")
add_executable(${tgt} ${name}.cpp)
Expand All @@ -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()
85 changes: 85 additions & 0 deletions tests/file-create-behavior.cpp
Original file line number Diff line number Diff line change
@@ -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 <cstdio>
#include <stdexcept>

/// 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;
}

0 comments on commit 40e71b2

Please sign in to comment.