switch templates to use in-memory buffers instead of copying from a file descriptor

also open with O_DIRECT

Author: calccrypto

include/template_db.h

@@ -75,13 +75,12 @@ extern "C" {
 #endif
 
 struct template_db {
-    int fd;
-    off_t size;
+    unsigned char *buf;
+    sqlite3_int64 size;
 };
 
 int init_template_db(struct template_db *tdb);
-int create_template(struct template_db *tdb, int (*create_tables)(const char *, sqlite3 *, void *),
-                    const char *name);
+int create_template(struct template_db *tdb, int (*create_tables)(const char *, sqlite3 *, void *));
 
 int create_xattr_tables(const char *name, sqlite3 *db, void *args);
 int create_xattrs_template(struct template_db *tdb);

src/template_db.c

@@ -62,14 +62,17 @@ OF SUCH DAMAGE.
 
 
 
+#ifndef _GNU_SOURCE
+#define _GNU_SOURCE /* O_DIRECT */
+#endif
+
 #include <errno.h>
 #include <fcntl.h>
 #include <stdio.h>
 #include <string.h>
 #include <sys/stat.h>
 #include <unistd.h>
 
-#include "dbutils.h"
 #include "external.h"
 #include "template_db.h"
 #include "utils.h"
@@ -78,7 +81,7 @@ OF SUCH DAMAGE.
 int init_template_db(struct template_db *tdb) {
     /* Not checking argument */
 
-    tdb->fd = -1;
+    tdb->buf = NULL;
     tdb->size = -1;
     return 0;
 }
@@ -112,75 +115,60 @@ int create_dbdb_tables(const char *name, sqlite3 *db, void *args) {
 int close_template_db(struct template_db *tdb) {
     /* Not checking argument */
 
-    close(tdb->fd);
+    sqlite3_free(tdb->buf);
     return init_template_db(tdb);
 }
 
 /* create the database file to copy from */
-int create_template(struct template_db *tdb, int (*create_tables)(const char *, sqlite3 *, void *),
-                    const char *name) {
-    if (!tdb || (tdb->fd != -1) ||
-        !create_tables || !name) {
+int create_template(struct template_db *tdb, int (*create_tables)(const char *, sqlite3 *, void *)) {
+    if (!tdb || tdb->buf ||
+        !create_tables) {
         return -1;
     }
 
-    sqlite3 *db = opendb(name, SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE, 0, 0, create_tables, NULL);
-
-    /*
-     * open before sqlite3_close to prevent potential race
-     * condition where file is deleted before being reopened
-     */
-    tdb->fd = open(name, O_RDONLY);
-
-    sqlite3_close(db);
-
-    /* no need for the file to remain on the filesystem */
-    remove(name);
-
-    if (tdb->fd == -1) {
-        fprintf(stderr, "Could not open template file '%s'\n", name);
-        return -1;
-    }
-
-    tdb->size = lseek(tdb->fd, 0, SEEK_END);
-    return !tdb->size;
+    sqlite3 *db = opendb(SQLITE_MEMORY, SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE, 0, 0, create_tables, NULL);
+    tdb->buf = sqlite3_serialize(db, "main", &tdb->size, 0);
+    closedb(db);
+    return !tdb->buf;
 }
 
 /* create the initial xattrs database file to copy from */
 int create_xattrs_template(struct template_db *tdb) {
-    static const char name[] = "xattrs_tmp.db";
-    return create_template(tdb, create_xattr_tables, name);
+    return create_template(tdb, create_xattr_tables);
 }
 
 /* create the initial main database file to copy from */
 int create_dbdb_template(struct template_db *tdb) {
-    static const char name[] = "tmp.db";
-    return create_template(tdb, create_dbdb_tables, name);
+    return create_template(tdb, create_dbdb_tables);
 }
 
 /* copy the template file instead of creating a new database and new tables for each work item */
 /* the ownership and permissions are set too */
 int copy_template(struct template_db *tdb, const char *dst, uid_t uid, gid_t gid) {
     /* Not checking arguments */
 
-    int err = 0;
-    const int src_db = dup(tdb->fd);
-    err = err?err:errno;
-    const int dst_db = open(dst, O_WRONLY | O_CREAT, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH);
-    err = err?err:errno;
-    const ssize_t sf = copyfd(src_db, 0, dst_db, 0, tdb->size);
-    err = err?err:errno;
-    fchown(dst_db, uid, gid);
-    err = err?err:errno;
-    close(src_db);
-    err = err?err:errno;
-    close(dst_db);
-    err = err?err:errno;
-
-    if (sf == -1) {
-        fprintf(stderr, "Could not copy template file (%d) to %s (%d): %s (%d)\n",
-                src_db, dst, dst_db, strerror(err), err);
-        remove(dst);
+    const int fd = open(dst, O_WRONLY | O_CREAT | O_DIRECT, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH);
+    if (fd < 0) {
+        return -1;
+    }
+
+    ssize_t written = 0;
+    while (written < tdb->size) {
+        const ssize_t w = pwrite(fd, tdb->buf + written, tdb->size - written, 0);
+        if (w < 1) {
+            break;
+        }
+
+        written += w;
+    }
+
+    close(fd);
+
+    if (written != tdb->size) {
+        return -1;
+    }
+
+    if (chown(dst, uid, gid) != 0) {
         return -1;
     }
 

test/unit/googletest/template_db.cpp

@@ -76,8 +76,6 @@ OF SUCH DAMAGE.
 #define TABLE_NAME "test_table"
 #define TABLE_CREATE "CREATE TABLE " TABLE_NAME "(A INT, B TEXT);"
 
-static const char template_name[] = "should-not-see-this-in-filesystem";
-
 static int sql_callback(void *args, int, char **data, char **) {
     std::string **cols = (std::string **) args;
     **cols = data[1];
@@ -99,15 +97,10 @@ TEST(template_db, create_copy) {
     struct template_db tdb;
     ASSERT_EQ(init_template_db(&tdb), 0);
 
-    ASSERT_EQ(create_template(&tdb, create_test_tables, template_name), 0);
-    EXPECT_GT(tdb.fd, -1);
+    ASSERT_EQ(create_template(&tdb, create_test_tables), 0);
+    EXPECT_NE(tdb.buf, nullptr);
     EXPECT_GT(tdb.size, 0);
 
-    // the template file should disappear even
-    // if it existed before this test
-    struct stat st;
-    EXPECT_EQ(lstat(template_name, &st), -1);
-
     char dst_name[7];
     snprintf(dst_name, sizeof(dst_name), "XXXXXX");
     int dst = mkstemp(dst_name);
@@ -135,26 +128,21 @@ TEST(template_db, bad_inputs) {
     struct template_db tdb;
     ASSERT_EQ(init_template_db(&tdb), 0);
 
-    EXPECT_EQ(create_template(nullptr, create_test_tables, template_name), -1);
-    EXPECT_EQ(create_template(&tdb,    nullptr,            template_name), -1);
-    EXPECT_EQ(create_template(&tdb,    create_test_tables, nullptr), -1);
-    EXPECT_EQ(create_template(&tdb,    create_test_tables, ""), -1);
-
-    tdb.fd = 0;
-    EXPECT_EQ(create_template(&tdb,    create_test_tables, template_name), -1);
-    tdb.fd = -1;
+    EXPECT_EQ(create_template(nullptr, create_test_tables), -1);
+    EXPECT_EQ(create_template(&tdb,    nullptr),            -1);
+    EXPECT_EQ(create_template(&tdb,    create_test_tables), -1);
 
-    EXPECT_EQ(copy_template(&tdb,      template_name, -1, -1), -1);
+    EXPECT_EQ(copy_template(&tdb,  "", -1, -1), -1);
 
-    EXPECT_EQ(template_to_db(&tdb,     template_name, -1, -1), nullptr);
+    EXPECT_EQ(template_to_db(&tdb, "", -1, -1), nullptr);
 
     ASSERT_EQ(close_template_db(&tdb), 0);
 }
 
 TEST(create_empty_dbdb, good) {
     struct template_db tdb;
     ASSERT_EQ(init_template_db(&tdb), 0);
-    ASSERT_EQ(create_template(&tdb, create_test_tables, template_name), 0);
+    ASSERT_EQ(create_template(&tdb, create_test_tables), 0);
 
     // create new directory
     char dirname[] = "XXXXXX" ;
@@ -198,10 +186,10 @@ TEST(create_empty_dbdb, good) {
     EXPECT_EQ(unlink(dbname), 0);
 
     // try to create db with a bad file descriptor
-    const int tfd = tdb.fd;
-    tdb.fd = -1;
+    unsigned char *tbuf = tdb.buf;
+    tdb.buf = nullptr;
     EXPECT_EQ(create_empty_dbdb(&tdb, &dst, -1, -1), -1);
-    tdb.fd = tfd;
+    tdb.buf = tbuf;
 
     // create new db
     EXPECT_EQ(create_empty_dbdb(&tdb, &dst, -1, -1), 0);