From ee4f0d4c8905af5fb94f50c372e68b8e246107ae Mon Sep 17 00:00:00 2001 From: Anna Schumaker Date: Wed, 30 Mar 2016 10:11:02 -0400 Subject: [PATCH] core/file: Writes go to a temporary file first And then rename the temporary file when closed. This protects users data in case Ocarina gets killed. Implements issue #31: Make file writes seem atomic Signed-off-by: Anna Schumaker --- core/file.c | 47 +++++++++++++++++++++++++++++++++++++++++++-- include/core/file.h | 12 +++++++++++- tests/core/file.c | 21 +++++++++++--------- 3 files changed, 68 insertions(+), 12 deletions(-) diff --git a/core/file.c b/core/file.c index 7a78379d..cf37f621 100644 --- a/core/file.c +++ b/core/file.c @@ -5,6 +5,7 @@ #include #include #include +#include #define REPORT_ERROR(fname) \ printf("%s (%s:%d): %s: %s\n", __func__, __FILE__, __LINE__, fname, strerror(errno)) @@ -35,6 +36,29 @@ static bool __file_mkdir() return ret == 0; } +static bool __file_can_write(struct file *file) +{ + gchar *path = file_path(file); + bool ret = true; + + if (g_access(path, F_OK) == 0 && g_access(path, W_OK) < 0) + ret = false; + + g_free(path); + return ret; +} + +static void __file_rename_tmp(struct file *file) +{ + gchar *path = file_path(file); + gchar *real = file_write_path(file); + + g_rename(real, path); + + g_free(real); + g_free(path); +} + void file_init(struct file *file, const gchar *name, unsigned int version) { @@ -52,6 +76,20 @@ gchar *file_path(struct file *file) return g_strdup(""); } +gchar *file_write_path(struct file *file) +{ + gchar *fname, *res; + + if (string_length(file->f_name) == 0) + return g_strdup(""); + + fname = g_strdup_printf(".%s.tmp", file->f_name); + res = __file_path(fname); + g_free(fname); + + return res; +} + const unsigned int file_version(struct file *file) { if (file->f_file && (file->f_mode == OPEN_READ)) @@ -85,8 +123,10 @@ static bool __file_open_write(struct file *file) { if (!__file_mkdir()) return false; + if (!__file_can_write(file)) + return false; - file->f_file = __file_open(file_path(file), "w"); + file->f_file = __file_open(file_write_path(file), "w"); if (!file->f_file) return false; @@ -106,8 +146,11 @@ bool file_open(struct file *file, enum open_mode mode) void file_close(struct file *file) { - if (file->f_file) + if (file->f_file) { fclose(file->f_file); + if (file->f_mode == OPEN_WRITE) + __file_rename_tmp(file); + } file->f_file = NULL; } diff --git a/include/core/file.h b/include/core/file.h index 994dee54..2d2a207c 100644 --- a/include/core/file.h +++ b/include/core/file.h @@ -59,6 +59,12 @@ void file_init(struct file *, const gchar *, unsigned int); */ gchar *file_path(struct file *); +/* + * Returns the path to the temporary file used for writes. + * This function allocates a new string that MUST be freed with g_free(). + */ +gchar *file_write_path(struct file *); + /* Returns the version number of the file. */ const unsigned int file_version(struct file *); @@ -75,13 +81,17 @@ bool file_exists(struct file *); * * When opening a file for writing (OPEN_WRITE): * - Create missing directories as needed. + * - Open a temporary file to protect data if Ocarina crashes. * - Write file->_version to the start of the file. * * Returns true if the open was successful and false otherwise. */ bool file_open(struct file *, enum open_mode); -/* Close an open file, setting file->f_file to NULL. */ +/* + * Closes an open file, setting file->f_file to NULL. If the file was opened + * with OPEN_WRITE, then rename the temporary file to file_path(). + */ void file_close(struct file *); /* diff --git a/tests/core/file.c b/tests/core/file.c index 78441fa2..ad7a6921 100644 --- a/tests/core/file.c +++ b/tests/core/file.c @@ -2,25 +2,23 @@ * Copyright 2014 (c) Anna Schumaker. */ #include +#include #include #include -static void test_verify_constructor(struct file *file, gchar *fpath) +static void test_verify_constructor(struct file *file, gchar *fpath, gchar *ftmp) { - gchar *path = file_path(file); - test_equal((void *)file->f_file, NULL); test_equal(file_version(file), 0); test_equal(file->f_mode, OPEN_READ); - test_equal(path, fpath); - - g_free(path); + test_str_equal(file_path(file), fpath); + test_str_equal(file_write_path(file), ftmp); } static void test_invalid_file(struct file *file) { - test_verify_constructor(file, ""); + test_verify_constructor(file, "", ""); test_equal(file_open(file, OPEN_READ), (bool)false); test_equal((void *)file->f_file, NULL); @@ -49,9 +47,13 @@ static void test_empty() static void test_file() { struct file file = FILE_INIT("file.txt", 0); - gchar *filepath = test_data_file("file.txt"); + gchar *basepath, *filepath, *realpath; - test_verify_constructor(&file, filepath); + basepath = g_strjoin("/", g_get_user_data_dir(), OCARINA_NAME, NULL); + filepath = g_strjoin("/", basepath, "file.txt", NULL); + realpath = g_strjoin("/", basepath, ".file.txt.tmp", NULL); + + test_verify_constructor(&file, filepath, realpath); test_equal(file_exists(&file), (bool)false); test_equal(test_data_file_exists(NULL), (bool)false); @@ -61,6 +63,7 @@ static void test_file() test_equal(file.f_mode, OPEN_WRITE); test_equal(file_open(&file, OPEN_WRITE), (bool)false); + test_equal(file_exists(&file), (bool)false); file_close(&file); test_equal((void *)file.f_file, NULL); test_equal(file.f_mode, OPEN_WRITE);