reftable/system: provide thin wrapper for tempfile subsystem

We use the tempfile subsystem to write temporary tables, but given that
we're in the process of converting the reftable library to become
standalone we cannot use this subsystem directly anymore. While we could
in theory convert the code to use mkstemp(3p) instead, we'd lose access
to our infrastructure that automatically prunes tempfiles via atexit(3p)
or signal handlers.

Provide a thin wrapper for the tempfile subsystem instead. Like this,
the compatibility shim is fully self-contained in "reftable/system.c".
Downstream users of the reftable library would have to implement their
own tempfile shims by replacing "system.c" with a custom version.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Makefile b/Makefile
index feeed6f9321a5069424c3adb21f92ad4abec82e8..50a79ad83fc0e4c458c4c86acf7ef8bac10c109b 100644 (file)
--- a/Makefile
+++ b/Makefile
@@ -2722,6 +2722,7 @@ REFTABLE_OBJS += reftable/pq.o
 REFTABLE_OBJS += reftable/reader.o
 REFTABLE_OBJS += reftable/record.o
 REFTABLE_OBJS += reftable/stack.o
+REFTABLE_OBJS += reftable/system.o
 REFTABLE_OBJS += reftable/tree.o
 REFTABLE_OBJS += reftable/writer.o
 

diff --git a/reftable/stack.c b/reftable/stack.c
index c67bdd952cac780c9b7e38eb490186af375eb337..2ac6a3715163c953c5cd2017d17392cbfbaf1e67 100644 (file)
--- a/reftable/stack.c
+++ b/reftable/stack.c
@@ -16,7 +16,6 @@ https://developers.google.com/open-source/licenses/bsd
 #include "reftable-record.h"
 #include "reftable-merged.h"
 #include "writer.h"
-#include "tempfile.h"
 
 static int stack_try_add(struct reftable_stack *st,
                         int (*write_table)(struct reftable_writer *wr,
@@ -867,7 +866,7 @@ int reftable_addition_add(struct reftable_addition *add,
        struct reftable_buf tab_file_name = REFTABLE_BUF_INIT;
        struct reftable_buf next_name = REFTABLE_BUF_INIT;
        struct reftable_writer *wr = NULL;
-       struct tempfile *tab_file = NULL;
+       struct reftable_tmpfile tab_file = REFTABLE_TMPFILE_INIT;
        struct fd_writer writer = {
                .opts = &add->stack->opts,
        };
@@ -887,20 +886,18 @@ int reftable_addition_add(struct reftable_addition *add,
        if (err < 0)
                goto done;
 
-       tab_file = mks_tempfile(temp_tab_file_name.buf);
-       if (!tab_file) {
-               err = REFTABLE_IO_ERROR;
+       err = tmpfile_from_pattern(&tab_file, temp_tab_file_name.buf);
+       if (err < 0)
                goto done;
-       }
        if (add->stack->opts.default_permissions) {
-               if (chmod(get_tempfile_path(tab_file),
+               if (chmod(tab_file.path,
                          add->stack->opts.default_permissions)) {
                        err = REFTABLE_IO_ERROR;
                        goto done;
                }
        }
 
-       writer.fd = get_tempfile_fd(tab_file);
+       writer.fd = tab_file.fd;
        err = reftable_writer_new(&wr, fd_writer_write, fd_writer_flush,
                                  &writer, &add->stack->opts);
        if (err < 0)
@@ -918,11 +915,9 @@ int reftable_addition_add(struct reftable_addition *add,
        if (err < 0)
                goto done;
 
-       err = close_tempfile_gently(tab_file);
-       if (err < 0) {
-               err = REFTABLE_IO_ERROR;
+       err = tmpfile_close(&tab_file);
+       if (err < 0)
                goto done;
-       }
 
        if (wr->min_update_index < add->next_update_index) {
                err = REFTABLE_API_ERROR;
@@ -945,11 +940,9 @@ int reftable_addition_add(struct reftable_addition *add,
          On windows, this relies on rand() picking a unique destination name.
          Maybe we should do retry loop as well?
         */
-       err = rename_tempfile(&tab_file, tab_file_name.buf);
-       if (err < 0) {
-               err = REFTABLE_IO_ERROR;
+       err = tmpfile_rename(&tab_file, tab_file_name.buf);
+       if (err < 0)
                goto done;
-       }
 
        REFTABLE_ALLOC_GROW(add->new_tables, add->new_tables_len + 1,
                            add->new_tables_cap);
@@ -960,7 +953,7 @@ int reftable_addition_add(struct reftable_addition *add,
        add->new_tables[add->new_tables_len++] = reftable_buf_detach(&next_name);
 
 done:
-       delete_tempfile(&tab_file);
+       tmpfile_delete(&tab_file);
        reftable_buf_release(&temp_tab_file_name);
        reftable_buf_release(&tab_file_name);
        reftable_buf_release(&next_name);
@@ -980,7 +973,7 @@ uint64_t reftable_stack_next_update_index(struct reftable_stack *st)
 static int stack_compact_locked(struct reftable_stack *st,
                                size_t first, size_t last,
                                struct reftable_log_expiry_config *config,
-                               struct tempfile **tab_file_out)
+                               struct reftable_tmpfile *tab_file_out)
 {
        struct reftable_buf next_name = REFTABLE_BUF_INIT;
        struct reftable_buf tab_file_path = REFTABLE_BUF_INIT;
@@ -988,7 +981,7 @@ static int stack_compact_locked(struct reftable_stack *st,
        struct fd_writer writer=  {
                .opts = &st->opts,
        };
-       struct tempfile *tab_file;
+       struct reftable_tmpfile tab_file = REFTABLE_TMPFILE_INIT;
        int err = 0;
 
        err = format_name(&next_name, reftable_reader_min_update_index(st->readers[first]),
@@ -1004,19 +997,17 @@ static int stack_compact_locked(struct reftable_stack *st,
        if (err < 0)
                goto done;
 
-       tab_file = mks_tempfile(tab_file_path.buf);
-       if (!tab_file) {
-               err = REFTABLE_IO_ERROR;
+       err = tmpfile_from_pattern(&tab_file, tab_file_path.buf);
+       if (err < 0)
                goto done;
-       }
 
        if (st->opts.default_permissions &&
-           chmod(get_tempfile_path(tab_file), st->opts.default_permissions) < 0) {
+           chmod(tab_file.path, st->opts.default_permissions) < 0) {
                err = REFTABLE_IO_ERROR;
                goto done;
        }
 
-       writer.fd = get_tempfile_fd(tab_file);
+       writer.fd = tab_file.fd;
        err = reftable_writer_new(&wr, fd_writer_write, fd_writer_flush,
                                  &writer, &st->opts);
        if (err < 0)
@@ -1030,15 +1021,15 @@ static int stack_compact_locked(struct reftable_stack *st,
        if (err < 0)
                goto done;
 
-       err = close_tempfile_gently(tab_file);
+       err = tmpfile_close(&tab_file);
        if (err < 0)
                goto done;
 
        *tab_file_out = tab_file;
-       tab_file = NULL;
+       tab_file = REFTABLE_TMPFILE_INIT;
 
 done:
-       delete_tempfile(&tab_file);
+       tmpfile_delete(&tab_file);
        reftable_writer_free(wr);
        reftable_buf_release(&next_name);
        reftable_buf_release(&tab_file_path);
@@ -1171,7 +1162,7 @@ static int stack_compact_range(struct reftable_stack *st,
        struct reftable_buf table_name = REFTABLE_BUF_INIT;
        struct lock_file tables_list_lock = LOCK_INIT;
        struct lock_file *table_locks = NULL;
-       struct tempfile *new_table = NULL;
+       struct reftable_tmpfile new_table = REFTABLE_TMPFILE_INIT;
        int is_empty_table = 0, err = 0;
        size_t first_to_replace, last_to_replace;
        size_t i, nlocks = 0;
@@ -1439,11 +1430,9 @@ static int stack_compact_range(struct reftable_stack *st,
                if (err < 0)
                        goto done;
 
-               err = rename_tempfile(&new_table, new_table_path.buf);
-               if (err < 0) {
-                       err = REFTABLE_IO_ERROR;
+               err = tmpfile_rename(&new_table, new_table_path.buf);
+               if (err < 0)
                        goto done;
-               }
        }
 
        /*
@@ -1515,7 +1504,7 @@ done:
                rollback_lock_file(&table_locks[i]);
        reftable_free(table_locks);
 
-       delete_tempfile(&new_table);
+       tmpfile_delete(&new_table);
        reftable_buf_release(&new_table_name);
        reftable_buf_release(&new_table_path);
        reftable_buf_release(&tables_list_buf);

diff --git a/reftable/system.c b/reftable/system.c
new file mode 100644 (file)
index 0000000..01f96f0
--- /dev/null
+++ b/reftable/system.c
@@ -0,0 +1,49 @@
+#include "system.h"
+#include "basics.h"
+#include "reftable-error.h"
+#include "../tempfile.h"
+
+int tmpfile_from_pattern(struct reftable_tmpfile *out, const char *pattern)
+{
+       struct tempfile *tempfile;
+
+       tempfile = mks_tempfile(pattern);
+       if (!tempfile)
+               return REFTABLE_IO_ERROR;
+
+       out->path = tempfile->filename.buf;
+       out->fd = tempfile->fd;
+       out->priv = tempfile;
+
+       return 0;
+}
+
+int tmpfile_close(struct reftable_tmpfile *t)
+{
+       struct tempfile *tempfile = t->priv;
+       int ret = close_tempfile_gently(tempfile);
+       t->fd = -1;
+       if (ret < 0)
+               return REFTABLE_IO_ERROR;
+       return 0;
+}
+
+int tmpfile_delete(struct reftable_tmpfile *t)
+{
+       struct tempfile *tempfile = t->priv;
+       int ret = delete_tempfile(&tempfile);
+       *t = REFTABLE_TMPFILE_INIT;
+       if (ret < 0)
+               return REFTABLE_IO_ERROR;
+       return 0;
+}
+
+int tmpfile_rename(struct reftable_tmpfile *t, const char *path)
+{
+       struct tempfile *tempfile = t->priv;
+       int ret = rename_tempfile(&tempfile, path);
+       *t = REFTABLE_TMPFILE_INIT;
+       if (ret < 0)
+               return REFTABLE_IO_ERROR;
+       return 0;
+}

diff --git a/reftable/system.h b/reftable/system.h
index 38d3534620e88e88dc370bdddc8d63816d34964f..858189fd55d91112b07923c2f4dec0f344559895 100644 (file)
--- a/reftable/system.h
+++ b/reftable/system.h
@@ -13,6 +13,46 @@ https://developers.google.com/open-source/licenses/bsd
 
 #include "git-compat-util.h"
 #include "lockfile.h"
-#include "tempfile.h"
+
+/*
+ * An implementation-specific temporary file. By making this specific to the
+ * implementation it becomes possible to tie temporary files into any kind of
+ * signal or atexit handlers for cleanup on abnormal situations.
+ */
+struct reftable_tmpfile {
+       const char *path;
+       int fd;
+       void *priv;
+};
+#define REFTABLE_TMPFILE_INIT ((struct reftable_tmpfile) { .fd = -1, })
+
+/*
+ * Create a temporary file from a pattern similar to how mkstemp(3p) would.
+ * The `pattern` shall not be modified. On success, the structure at `out` has
+ * been initialized such that it is ready for use. Returns 0 on success, a
+ * reftable error code on error.
+ */
+int tmpfile_from_pattern(struct reftable_tmpfile *out, const char *pattern);
+
+/*
+ * Close the temporary file's file descriptor without removing the file itself.
+ * This is a no-op in case the file has already been closed beforehand. Returns
+ * 0 on success, a reftable error code on error.
+ */
+int tmpfile_close(struct reftable_tmpfile *t);
+
+/*
+ * Close the temporary file and delete it. This is a no-op in case the file has
+ * already been deleted or renamed beforehand. Returns 0 on success, a reftable
+ * error code on error.
+ */
+int tmpfile_delete(struct reftable_tmpfile *t);
+
+/*
+ * Rename the temporary file to the provided path. The temporary file must be
+ * active. Return 0 on success, a reftable error code on error. Deactivates the
+ * temporary file.
+ */
+int tmpfile_rename(struct reftable_tmpfile *t, const char *path);
 
 #endif