compact: preserve backup database until compacted database is in place

It is less error prone and window of failure opportunity is smaller
if the old (backup) database is always renamed (instead of sometimes
rmtree'd) before new (compacted) database is put into its place.
Finally rmtree() old database in case old database backup is not kept.

diff --git a/lib/database.cc b/lib/database.cc
index 3530cb650afba31748ea6fe9a15221fd8943ba16..d79cc3006d29a204aaa54a762aa30e0a4734ded7 100644 (file)
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -873,6 +873,7 @@ notmuch_database_compact (const char *path,
     notmuch_status_t ret = NOTMUCH_STATUS_SUCCESS;
     notmuch_database_t *notmuch = NULL;
     struct stat statbuf;
+    notmuch_bool_t keep_backup;
 
     local = talloc_new (NULL);
     if (! local)
@@ -898,17 +899,27 @@ 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, "Path already exists: %s\n", backup_path);
+       ret = NOTMUCH_STATUS_FILE_ERROR;
+       goto DONE;
+    }
+    if (errno != ENOENT) {
+       fprintf (stderr, "Unknown error while stat()ing path: %s\n",
+                strerror (errno));
+       ret = NOTMUCH_STATUS_FILE_ERROR;
+       goto DONE;
     }
 
     try {
@@ -924,14 +935,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)) {
@@ -940,6 +947,9 @@ notmuch_database_compact (const char *path,
        goto DONE;
     }
 
+    if (! keep_backup)
+       rmtree (backup_path);
+
   DONE:
     if (notmuch)
        notmuch_database_destroy (notmuch);