Re: [PATCH v2 3/5] compact: preserve backup database until compacted database is in place

[Jani Nikula]

On Wed, 13 Nov 2013, Tomi Ollila tomi.oll...@iki.fi wrote:
 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.
 ---
  lib/database.cc | 42 +-
  1 file changed, 25 insertions(+), 17 deletions(-)

 diff --git a/lib/database.cc b/lib/database.cc
 index 3530cb6..ee09c5e 100644
 --- 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,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;

*cough* I thought you were the style police around here *cough*

 +
 +if (stat (backup_path, statbuf) != -1) {
 + fprintf (stderr, Backup path already exists: %s\n, backup_path);

The user will be confused if he specifically didn't add --backup and
happens to get this. But maybe it's a corner case. *shrug*.

 + ret = NOTMUCH_STATUS_FILE_ERROR;
 + goto DONE;
 +}
 +if (errno != ENOENT) {
 + fprintf (stderr, Unknown error while stat()ing backup path: %s\n,
 +  strerror (errno));

ret = ?

 + goto DONE;
  }
  
  try {
 @@ -924,14 +933,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 +945,9 @@ notmuch_database_compact (const char *path,
   goto DONE;
  }
  
 +if (! keep_backup)
 + rmtree (backup_path);
 +
DONE:
  if (notmuch)
   notmuch_database_destroy (notmuch);
 -- 
 1.8.3.1

 ___
 notmuch mailing list
 notmuch@notmuchmail.org
 http://notmuchmail.org/mailman/listinfo/notmuch
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH v2 3/5] compact: preserve backup database until compacted database is in place

[Tomi Ollila]

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.
---
 lib/database.cc | 42 +-
 1 file changed, 25 insertions(+), 17 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index 3530cb6..ee09c5e 100644
--- 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,25 @@ notmuch_database_compact (const char *path,
goto DONE;
 }

-if (backup_path != NULL) {
-   if (stat (backup_path, ) != -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, ) != -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 {
@@ -924,14 +933,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 +945,9 @@ notmuch_database_compact (const char *path,
goto DONE;
 }

+if (! keep_backup)
+   rmtree (backup_path);
+
   DONE:
 if (notmuch)
notmuch_database_destroy (notmuch);
-- 
1.8.3.1