Fix enough bugs to run a simple test

Signed-off-by: Cole Miller <[email protected]>

src/vfs2.c

@@ -109,6 +109,8 @@ struct vfs2_wal
 	dqlite_vfs_frame *pending_txn_frames;   /* for vfs2_poll */
 	uint32_t pending_txn_last_frame_commit; /* commit marker for the
 						   physical last frame */
+
+	bool is_empty;
 };
 
 struct vfs2_db
@@ -342,6 +344,15 @@ static int vfs2_wal_swap(struct vfs2_file *wal, const struct vfs2_wal_hdr *hdr)
 	assert(hdr->salt[1] == region0->hdr.basic[0].aCksum[1]);
 	db->db_shm.prev_txn_hdr = region0->hdr.basic[0];
 
+	dqlite_vfs_frame *frames = wal->wal.pending_txn_frames;
+	uint32_t n = wal->wal.pending_txn_len;
+	for (uint32_t i = 0; i < n; i++) {
+		sqlite3_free(frames[i].data);
+	}
+	sqlite3_free(frames);
+	wal->wal.pending_txn_frames = NULL;
+	wal->wal.pending_txn_len = 0;
+
 	/* Move the moving name. */
 	rv = unlink(wal->wal.moving_name);
 	if (rv != 0) {
@@ -353,7 +364,7 @@ static int vfs2_wal_swap(struct vfs2_file *wal, const struct vfs2_wal_hdr *hdr)
 	}
 
 	(void)phys_outgoing->pMethods->xWrite(phys_outgoing, &invalid_magic,
-					       sizeof(invalid_magic), 0);
+					      sizeof(invalid_magic), 0);
 	return SQLITE_OK;
 }
 
@@ -369,12 +380,10 @@ static int vfs2_wal_write_frame_hdr(struct vfs2_file *wal,
 		/* Appending to the end of the WAL. */
 		if (wal->wal.pending_txn_last_frame_commit > 0) {
 			/* Start of a new transaction. */
-			if (frames != NULL) {
-				for (uint32_t i = 0; i < n; i++) {
-					sqlite3_free(frames[i].data);
-				}
-				sqlite3_free(frames);
+			for (uint32_t i = 0; i < n; i++) {
+				sqlite3_free(frames[i].data);
 			}
+			sqlite3_free(frames);
 			wal->wal.pending_txn_frames = NULL;
 			wal->wal.pending_txn_start += n;
 			wal->wal.pending_txn_len = 0;
@@ -386,7 +395,7 @@ static int vfs2_wal_write_frame_hdr(struct vfs2_file *wal,
 		if (wal->wal.pending_txn_frames == NULL) {
 			return SQLITE_NOMEM;
 		}
-		dqlite_vfs_frame *frame = &frames[n];
+		dqlite_vfs_frame *frame = &wal->wal.pending_txn_frames[n];
 		frame->page_number = ByteGetBe32(buf);
 		frame->data = NULL;
 		wal->wal.pending_txn_last_frame_commit =
@@ -417,8 +426,8 @@ static int vfs2_wal_post_write(struct vfs2_file *wal,
 		x /= frame_size;
 		return vfs2_wal_write_frame_hdr(wal, buf, x);
 	} else if (amt == (int)wal->vfs_data->page_size) {
-		sqlite3_int64 x =
-		    ofst - VFS2_WAL_FRAME_HDR_SIZE - sizeof(struct vfs2_wal_hdr);
+		sqlite3_int64 x = ofst - VFS2_WAL_FRAME_HDR_SIZE -
+				  sizeof(struct vfs2_wal_hdr);
 		assert(x % frame_size == 0);
 		x /= frame_size;
 		x -= wal->wal.pending_txn_start;
@@ -444,7 +453,11 @@ static int vfs2_write(sqlite3_file *file,
 
 	if ((xfile->flags & SQLITE_OPEN_WAL) && ofst == 0) {
 		assert(amt == sizeof(struct vfs2_wal_hdr));
-		return vfs2_wal_swap(xfile, buf);
+		bool is_empty = xfile->wal.is_empty;
+		xfile->wal.is_empty = false;
+		if (!is_empty) {
+			return vfs2_wal_swap(xfile, buf);
+		}
 	}
 
 	rv = xfile->orig->pMethods->xWrite(xfile->orig, buf, amt, ofst);
@@ -719,7 +732,11 @@ static struct sqlite3_io_methods vfs2_io_methods = {
 
 /* sqlite3_vfs implementations begin here */
 
-static int compare_wals(sqlite3_file *xwal1, sqlite3_file *xwal2, bool *invert) {
+static int compare_wals(sqlite3_file *xwal1,
+			sqlite3_file *xwal2,
+			bool *invert,
+			bool *empty)
+{
 	int rv;
 	sqlite3_int64 size1;
 	rv = xwal1->pMethods->xFileSize(xwal1, &size1);
@@ -732,6 +749,10 @@ static int compare_wals(sqlite3_file *xwal1, sqlite3_file *xwal2, bool *invert)
 		return rv;
 	}
 
+	if (size1 == 0 && size2 == 0) {
+		*empty = true;
+	}
+
 	if (size2 == 0) {
 		*invert = false;
 	} else if (size1 == 0) {
@@ -758,7 +779,6 @@ static int compare_wals(sqlite3_file *xwal1, sqlite3_file *xwal2, bool *invert)
 		}
 	}
 
-
 	return SQLITE_OK;
 }
 
@@ -812,7 +832,7 @@ static int vfs2_open_wal(sqlite3_vfs *vfs,
 	}
 
 	bool invert;
-	rv = compare_wals(phys1, phys2, &invert);
+	rv = compare_wals(phys1, phys2, &invert, &xout->wal.is_empty);
 	if (invert) {
 		rv = unlink(name);
 		(void)rv;
@@ -845,6 +865,8 @@ static int vfs2_open_wal(sqlite3_vfs *vfs,
 		}
 	}
 
+	xout->wal.is_empty = true;
+
 	register_file(xout, &entry);
 	sqlite3_free(entry);
 	return SQLITE_OK;

test/unit/test_vfs2.c

@@ -1,5 +1,6 @@
 #include <errno.h>
 #include <stdio.h>
+#include <sys/stat.h>
 #include <unistd.h>
 
 #include <raft.h>
@@ -373,6 +374,95 @@ TEST(vfs2_open, tmp, setUp, tearDown, 0, NULL)
 	return MUNIT_OK;
 }
 
+/* Various edge cases of opening a WAL */
+TEST(vfs2_open, wal_weirdness, setUp, tearDown, 0, NULL)
+{
+	(void)params;
+	int rc;
+	struct fixture *f = data;
+	sqlite3_file *db = munit_malloc(f->vfs->szOsFile);
+	sqlite3_file *wal = munit_malloc(f->vfs->szOsFile);
+	int flags;
+
+	vfsFillPath(f, "foo.db");
+	rc = f->vfs->xOpen(
+	    f->vfs, f->path, db,
+	    SQLITE_OPEN_CREATE | SQLITE_OPEN_READWRITE | SQLITE_OPEN_MAIN_DB,
+	    &flags);
+	munit_assert_int(rc, ==, SQLITE_OK);
+
+	vfsFillPath(f, "foo.db-wal");
+	rc = f->vfs->xOpen(
+	    f->vfs, f->path, wal,
+	    SQLITE_OPEN_CREATE | SQLITE_OPEN_READWRITE | SQLITE_OPEN_WAL,
+	    &flags);
+	munit_assert_int(rc, ==, SQLITE_OK);
+
+	/* Now the two physical WALs and the moving name exist. The moving name
+	 * points to -xwal1. */
+	struct stat st;
+	rc = stat(f->path, &st);
+	munit_assert_int(rc, ==, 0);
+	unsigned long long ino = st.st_ino;
+	char fixed1[VFS_PATH_SZ];
+	snprintf(fixed1, VFS_PATH_SZ, "%s/%s", f->dir, "foo.db-xwal1");
+	rc = stat(fixed1, &st);
+	munit_assert_int(rc, ==, 0);
+	unsigned long long ino1 = st.st_ino;
+	munit_assert_ullong(ino, ==, ino1);
+	munit_assert_ullong(st.st_size, ==, 0);
+	char fixed2[VFS_PATH_SZ];
+	snprintf(fixed2, VFS_PATH_SZ, "%s/%s", f->dir, "foo.db-xwal2");
+	rc = stat(fixed2, &st);
+	munit_assert_int(rc, ==, 0);
+	munit_assert_ullong(st.st_size, ==, 0);
+	unsigned long ino2 = st.st_ino;
+
+	/* Write the header for the first time. This goes to xwal1. The moving
+	 * name doesn't move. */
+	char hdr[32] = {0};
+	rc = wal->pMethods->xWrite(wal, hdr, sizeof(hdr), 0);
+	munit_assert_int(rc, ==, SQLITE_OK);
+	rc = stat(fixed1, &st);
+	munit_assert_ullong(st.st_size, ==, sizeof(hdr));
+	rc = stat(f->path, &st);
+	munit_assert_int(rc, ==, 0);
+	munit_assert_ullong(st.st_ino, ==, ino1);
+
+	char buf[24 + 512];
+	/* Write a frame to the WAL. */
+	rc = wal->pMethods->xWrite(wal, buf, 24, 32);
+	munit_assert_int(rc, ==, 0);
+	rc = wal->pMethods->xWrite(wal, buf + 24, 512, 32 + 24);
+	munit_assert_int(rc, ==, 0);
+
+	/* Force allocation of the WAL index. */
+	volatile void *p;
+	rc = db->pMethods->xShmMap(db, 0, 1 << 15, 1, &p);
+	munit_assert_int(rc, ==, 0);
+
+	/* Write the header again. This triggers a real WAL swap. */
+	rc = wal->pMethods->xWrite(wal, hdr, sizeof(hdr), 0);
+	munit_assert_int(rc, ==, SQLITE_OK);
+	rc = stat(fixed1, &st);
+	munit_assert_int(rc, ==, 0);
+	munit_assert_ullong(st.st_size, ==, sizeof(hdr) + sizeof(buf));
+	rc = stat(fixed2, &st);
+	munit_assert_int(rc, ==, 0);
+	munit_assert_ullong(st.st_size, ==, sizeof(hdr));
+	rc = stat(f->path, &st);
+	munit_assert_int(rc, ==, 0);
+	munit_assert_ullong(st.st_ino, ==, ino2);
+
+	wal->pMethods->xClose(wal);
+	db->pMethods->xClose(db);
+
+	free(wal);
+	free(db);
+
+	return MUNIT_OK;
+}
+
 /******************************************************************************
  *
  * xDelete