Minor touch-ups

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

Makefile.am

@@ -177,6 +177,7 @@ unit_test_SOURCES += \
   test/unit/test_sm.c \
   test/unit/test_tuple.c \
   test/unit/test_vfs.c \
+  test/unit/test_vfs_extra.c \
   test/unit/test_vfs2.c \
   test/unit/main.c
 unit_test_CFLAGS = $(AM_CFLAGS) -Wno-unknown-warning-option -Wno-uninitialized -Wno-maybe-uninitialized -Wno-float-equal -Wno-conversion
@@ -195,7 +196,6 @@ integration_test_SOURCES = \
   test/integration/test_node.c \
   test/integration/test_role_management.c \
   test/integration/test_server.c \
-  test/integration/test_vfs.c \
   test/integration/main.c
 integration_test_CFLAGS = $(AM_CFLAGS) -Wno-conversion
 integration_test_LDFLAGS = $(AM_LDFLAGS) -no-install

include/dqlite.h

@@ -743,13 +743,6 @@ DQLITE_API int dqlite_vfs_num_pages(sqlite3_vfs *vfs,
 				    const char *filename,
 				    unsigned *n);
 
-/**
- * Return the number of alive database pages.
- */
-DQLITE_API int dqlite_vfs_num_alive_pages(sqlite3_vfs *vfs,
-				    const char *filename,
-				    unsigned *n);
-
 /**
  * This function is DEPRECATED and will be removed in a future major release.
  *

src/dqlite.c

@@ -52,35 +52,11 @@ int dqlite_vfs_snapshot(sqlite3_vfs *vfs,
 	return VfsSnapshot(vfs, filename, data, n);
 }
 
-int dqlite_vfs_snapshot_disk(sqlite3_vfs *vfs,
-			     const char *filename,
-			     struct dqlite_buffer bufs[],
-			     unsigned n)
-{
-	int rv;
-	if (n != 2) {
-		return -1;
-	}
-
-	rv = VfsDiskSnapshotDb(vfs, filename, &bufs[0]);
-	if (rv != 0) {
-		return rv;
-	}
-
-	rv = VfsDiskSnapshotWal(vfs, filename, &bufs[1]);
-	return rv;
-}
-
 int dqlite_vfs_num_pages(sqlite3_vfs *vfs, const char *filename, unsigned *n)
 {
 	return VfsDatabaseNumPages(vfs, filename, 0, n);
 }
 
-int dqlite_vfs_num_alive_pages(sqlite3_vfs *vfs, const char *filename, unsigned *n)
-{
-	return VfsDatabaseNumPages(vfs, filename, 1, n);
-}
-
 int dqlite_vfs_shallow_snapshot(sqlite3_vfs *vfs,
 				const char *filename,
 				struct dqlite_buffer bufs[],

src/fsm.c

@@ -391,8 +391,8 @@ static int encodeDatabase(struct db *db,
 
 	header.filename = db->filename;
 	header.main_size = (n - 1) * (uint64_t)db->config->page_size;
-	// The database is checkpointed before writing it to disk.
-	// As such, wal_size is always 0.
+	/* The database is checkpointed before writing it to disk.  As such,
+	 * wal_size is always 0. */
 	header.wal_size = 0;
 
 	vfs = sqlite3_vfs_find(db->config->name);
@@ -478,7 +478,7 @@ static unsigned dbNumPages(struct db *db)
 	uint32_t n;
 
 	vfs = sqlite3_vfs_find(db->config->name);
-	rv = VfsDatabaseNumPages(vfs, db->filename, 1, &n);
+	rv = VfsDatabaseNumPages(vfs, db->filename, true, &n);
 	assert(rv == 0);
 	return n;
 }

src/vfs.c

@@ -1228,16 +1228,16 @@ static int vfsFileCheckReservedLock(sqlite3_file *file, int *result)
 
 /* Handle pragma a pragma file control. See the xFileControl
  * docstring in sqlite.h.in for more details. */
-static int vfsFileControlPragma(struct vfsFile *f, char **fnctl)
+static int vfsFileControlPragma(struct vfsFile *f, char **fcntl)
 {
 	const char *left;
 	const char *right;
 
 	assert(f != NULL);
-	assert(fnctl != NULL);
+	assert(fcntl != NULL);
 
-	left = fnctl[1];
-	right = fnctl[2];
+	left = fcntl[1];
+	right = fcntl[2];
 
 	assert(left != NULL);
 
@@ -1261,7 +1261,7 @@ static int vfsFileControlPragma(struct vfsFile *f, char **fnctl)
 			if (f->database->n_pages > 0 &&
 			    page_size !=
 				(int)vfsDatabaseGetPageSize(f->database)) {
-				fnctl[0] = sqlite3_mprintf(
+				fcntl[0] = sqlite3_mprintf(
 				    "changing page size is not supported");
 				return SQLITE_IOERR;
 			}
@@ -1270,13 +1270,13 @@ static int vfsFileControlPragma(struct vfsFile *f, char **fnctl)
 		/* When the user executes 'PRAGMA journal_mode=x' we ensure
 		 * that the desired mode is 'wal'. */
 		if (strcasecmp(right, "wal") != 0) {
-			fnctl[0] =
+			fcntl[0] =
 			    sqlite3_mprintf("only WAL mode is supported");
 			return SQLITE_IOERR;
 		}
 	} else if (sqlite3_stricmp(left, "wal_checkpoint") == 0 
 			|| (sqlite3_stricmp(left, "wal_autocheckpoint") == 0 && right)) {
-		fnctl[0] = sqlite3_mprintf("custom checkpoint not allowed");
+		fcntl[0] = sqlite3_mprintf("custom checkpoint not allowed");
 		return SQLITE_IOERR;
 	}
 
@@ -2453,30 +2453,34 @@ static uint32_t vfsDatabaseGetNumberOfPages(struct vfsDatabase *d)
 	return ByteGetBe32(&page[28]);
 }
 
-static uint32_t vfsDatabaseNumPages(struct vfsDatabase *database, int useWal) {
+static uint32_t vfsDatabaseNumPages(struct vfsDatabase *database, bool use_wal) {
 	uint32_t n;
-	if (useWal && database->wal.n_frames) {
+	if (use_wal && database->wal.n_frames > 0) {
 		n = vfsFrameGetDatabaseSize(database->wal.frames[database->wal.n_frames-1]);
-		// If the result is zero, it means that the WAL contains uncommitted transactions.
-		assert(n != 0);
+		/* If the result is zero, it means that the WAL contains
+		 * uncommitted transactions. */
+		POST(n > 0);
 	} else {
 		n = vfsDatabaseGetNumberOfPages(database);
 	}
 	return n;
 }
 
-int VfsDatabaseNumPages(sqlite3_vfs *vfs, const char *filename, int useWal, uint32_t *n)
+int VfsDatabaseNumPages(sqlite3_vfs *vfs,
+			const char *filename,
+			bool use_wal,
+			uint32_t *n)
 {
 	struct vfs *v;
 	struct vfsDatabase *d;
-	
+
 	v = (struct vfs *)(vfs->pAppData);
 	d = vfsDatabaseLookup(v, filename);
 	if (d == NULL) {
 		return -1;
 	}
 
-	*n = vfsDatabaseNumPages(d, useWal);
+	*n = vfsDatabaseNumPages(d, use_wal);
 	return 0;
 }
 
@@ -2581,7 +2585,8 @@ static void vfsDatabaseShallowSnapshot(struct vfsDatabase *d,
 }
 
 static void vfsWalShallowSnapshot(struct vfsWal *w,
-				       struct dqlite_buffer *bufs, uint32_t n) {
+				  struct dqlite_buffer *bufs,
+				  uint32_t n) {
 	uint32_t page_size;
 	unsigned i;
 
@@ -2596,8 +2601,8 @@ static void vfsWalShallowSnapshot(struct vfsWal *w,
 		struct vfsFrame *frame = w->frames[i];
 		uint32_t page_number = vfsFrameGetPageNumber(frame);
 		assert(page_number <= n);
-		bufs[page_number-1].base = frame->page;
-		bufs[page_number-1].len = page_size;
+		bufs[page_number - 1].base = frame->page;
+		bufs[page_number - 1].len = page_size;
 	}
 }
 
@@ -2623,15 +2628,13 @@ int VfsShallowSnapshot(sqlite3_vfs *vfs,
 		return SQLITE_CORRUPT;
 	}
 
-	if (vfsDatabaseNumPages(database, 1) != n) {
+	if (vfsDatabaseNumPages(database, true) != n) {
 		tracef("not enough buffers provided");
 		return SQLITE_MISUSE;
 	}
 
-	/* Copy page pointers to first n buffers */
 	vfsDatabaseShallowSnapshot(database, bufs, n);
-
-	/* Copy page pointers from wal */
+	/* Update the array of pages by  */
 	vfsWalShallowSnapshot(&database->wal, bufs, n);
 
 	return 0;
@@ -3053,17 +3056,17 @@ static int vfsDiskFileCheckReservedLock(sqlite3_file *file, int *result)
 
 /* Handle pragma a pragma file control. See the xFileControl
  * docstring in sqlite.h.in for more details. */
-static int vfsDiskFileControlPragma(struct vfsFile *f, char **fnctl)
+static int vfsDiskFileControlPragma(struct vfsFile *f, char **fcntl)
 {
 	int rv;
 	const char *left;
 	const char *right;
 
 	assert(f != NULL);
-	assert(fnctl != NULL);
+	assert(fcntl != NULL);
 
-	left = fnctl[1];
-	right = fnctl[2];
+	left = fcntl[1];
+	right = fcntl[2];
 
 	assert(left != NULL);
 
@@ -3074,30 +3077,30 @@ static int vfsDiskFileControlPragma(struct vfsFile *f, char **fnctl)
 		 * Only used for on-disk databases.
 		 * */
 		if (f->db == NULL) {
-			fnctl[0] = sqlite3_mprintf("no DB file found");
+			fcntl[0] = sqlite3_mprintf("no DB file found");
 			return SQLITE_IOERR;
 		}
 		if (page_size > UINT16_MAX) {
-			fnctl[0] = sqlite3_mprintf("max page_size exceeded");
+			fcntl[0] = sqlite3_mprintf("max page_size exceeded");
 			return SQLITE_IOERR;
 		}
 		if (f->database->page_size == 0) {
 			rv = f->db->pMethods->xFileControl(
-			    f->db, SQLITE_FCNTL_PRAGMA, fnctl);
+			    f->db, SQLITE_FCNTL_PRAGMA, fcntl);
 			if (rv == SQLITE_NOTFOUND || rv == SQLITE_OK) {
 				f->database->page_size = (uint16_t)page_size;
 			}
 			return rv;
 		} else if ((uint16_t)page_size != f->database->page_size) {
-			fnctl[0] = sqlite3_mprintf(
+			fcntl[0] = sqlite3_mprintf(
 			    "changing page size is not supported");
 			return SQLITE_IOERR;
 		}
 	} else if (strcmp(left, "journal_mode") == 0 && right) {
 		/* When the user executes 'PRAGMA journal_mode=x' we ensure
 		 * that the desired mode is 'wal'. */
 		if (strcasecmp(right, "wal") != 0) {
-			fnctl[0] =
+			fcntl[0] =
 			    sqlite3_mprintf("only WAL mode is supported");
 			return SQLITE_IOERR;
 		}
@@ -3515,6 +3518,25 @@ int VfsDiskSnapshotDb(sqlite3_vfs *vfs,
 	return rv;
 }
 
+int VfsSnapshotDisk(sqlite3_vfs *vfs,
+		    const char *filename,
+		    struct dqlite_buffer bufs[],
+		    unsigned n)
+{
+	int rv;
+	if (n != 2) {
+		return -1;
+	}
+
+	rv = VfsDiskSnapshotDb(vfs, filename, &bufs[0]);
+	if (rv != 0) {
+		return rv;
+	}
+
+	rv = VfsDiskSnapshotWal(vfs, filename, &bufs[1]);
+	return rv;
+}
+
 static int vfsDiskDatabaseRestore(struct vfsDatabase *d,
 				  const char *filename,
 				  const uint8_t *data,

src/vfs.h

@@ -38,11 +38,15 @@ int VfsAbort(sqlite3_vfs *vfs, const char *filename);
 /* Make a full snapshot of a database. */
 int VfsSnapshot(sqlite3_vfs *vfs, const char *filename, void **data, size_t *n);
 
-/* Makes a full, shallow snapshot of a database. The first n-1 buffers will each
- * contain a pointer to the actual database pages, while the n'th buffer
- * will contain a copy of the WAL. `bufs` MUST point to an array of n
- * `dqlite_buffer` structs and n MUST equal 1 + the number of pages in
- * the database. */
+/**
+ * Prepare a snapshot of the selected database, borrowing from the in-memory
+ * state of the VFS.
+ *
+ * The provided array of buffers will be populated with pointers to the
+ * in-memory database held by the VFS. It's forbidden to checkpoint the
+ * database while these pointers are still in use. VfsDatabaseNumPages (with
+ * `use_wal = true`) should be used to determine how many buffers are needed.
+ */
 int VfsShallowSnapshot(sqlite3_vfs *vfs,
 		       const char *filename,
 		       struct dqlite_buffer bufs[],
@@ -58,6 +62,11 @@ int VfsDiskSnapshotDb(sqlite3_vfs *vfs,
 		      const char *path,
 		      struct dqlite_buffer *buf);
 
+int VfsSnapshotDisk(sqlite3_vfs *vfs,
+		    const char *filename,
+		    struct dqlite_buffer bufs[],
+		    uint32_t n);
+
 /* Restore a database snapshot. */
 int VfsRestore(sqlite3_vfs *vfs,
 	       const char *filename,
@@ -71,8 +80,16 @@ int VfsDiskRestore(sqlite3_vfs *vfs,
 		   size_t main_size,
 		   size_t wal_size);
 
-/* Number of pages in the database. */
-int VfsDatabaseNumPages(sqlite3_vfs *vfs, const char *filename, int useWal, uint32_t *n);
+/**
+ * Number of pages in the database.
+ *
+ * If `use_wal` is set, returns the number of pages that the database would have
+ * after fully checkpointing the WAL.
+ */
+int VfsDatabaseNumPages(sqlite3_vfs *vfs,
+			const char *filename,
+			bool use_wal,
+			uint32_t *n);
 
 /* Returns the resulting size of the main file, wal file and n additional WAL
  * frames with the specified page_size. */