[PATCH 1/3] lib: make compact to keep backup until new database in place

[Tomi Ollila]

It is less error prone and window of failure opportunity is smaller
if the old database is always renamed (instead of sometimes rmtree'd)
before new database is put into its place.
Finally rmtree() old database in case old database backup is not kept.
---
 lib/database.cc | 44 ++++++++++++++++++++++++++------------------
 1 file changed, 26 insertions(+), 18 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index a021bf1..6b656e9 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -871,6 +871,7 @@ notmuch_database_compact (const char* path,
     notmuch_status_t ret = NOTMUCH_STATUS_SUCCESS;
     notmuch_database_t *notmuch = NULL;
     struct stat statbuf;
+    bool keep_backup;
 
     local = talloc_new (NULL);
     if (! local)
@@ -896,17 +897,25 @@ notmuch_database_compact (const char* path,
 	goto DONE;
     }
 
-    if (backup_path != NULL) {
-	if (stat(backup_path, &statbuf) != -1) {
-	    fprintf (stderr, "Backup path already exists: %s\n", backup_path);
-	    ret = NOTMUCH_STATUS_FILE_ERROR;
-	    goto DONE;
-	}
-	if (errno != ENOENT) {
-	    fprintf (stderr, "Unknown error while stat()ing backup path: %s\n",
-		     strerror(errno));
+    if (backup_path == NULL) {
+	if (! (backup_path = talloc_asprintf (local, "%s.old", xapian_path))) {
+	    ret = NOTMUCH_STATUS_OUT_OF_MEMORY;
 	    goto DONE;
 	}
+	keep_backup = FALSE;
+    }
+    else
+	keep_backup = TRUE;
+
+    if (stat (backup_path, &statbuf) != -1) {
+	fprintf (stderr, "Backup path already exists: %s\n", backup_path);
+	ret = NOTMUCH_STATUS_FILE_ERROR;
+	goto DONE;
+    }
+    if (errno != ENOENT) {
+	fprintf (stderr, "Unknown error while stat()ing backup path: %s\n",
+		 strerror(errno));
+	goto DONE;
     }
 
     try {
@@ -922,14 +931,10 @@ notmuch_database_compact (const char* path,
 	goto DONE;
     }
 
-    if (backup_path) {
-	if (rename(xapian_path, backup_path)) {
-	    fprintf (stderr, "Error moving old database out of the way\n");
-	    ret = NOTMUCH_STATUS_FILE_ERROR;
-	    goto DONE;
-	}
-    } else {
-	rmtree(xapian_path);
+    if (rename(xapian_path, backup_path)) {
+	fprintf (stderr, "Error moving old database out of the way\n");
+	ret = NOTMUCH_STATUS_FILE_ERROR;
+	goto DONE;
     }
 
     if (rename(compact_xapian_path, xapian_path)) {
@@ -938,6 +943,9 @@ notmuch_database_compact (const char* path,
 	goto DONE;
     }
 
+    if (! keep_backup)
+	rmtree (backup_path);
+
 DONE:
     if (notmuch)
 	notmuch_database_destroy (notmuch);
@@ -1538,7 +1546,7 @@ _notmuch_database_generate_doc_id (notmuch_database_t *notmuch)
     notmuch->last_doc_id++;
 
     if (notmuch->last_doc_id == 0)
-	INTERNAL_ERROR ("Xapian document IDs are exhausted.\n");	
+	INTERNAL_ERROR ("Xapian document IDs are exhausted.\n");
 
     return notmuch->last_doc_id;
 }
-- 
1.8.3.1