From: Austin Clements <amdragon@MIT.EDU>
To: notmuch@notmuchmail.org
Subject: [PATCH v2 1/6] lib/cli: Make notmuch_database_open return a status
	code
Date: Mon, 30 Apr 2012 12:25:33 -0400
Message-Id: <1335803138-24024-2-git-send-email-amdragon@mit.edu>
In-Reply-To: <1335803138-24024-1-git-send-email-amdragon@mit.edu>
References: <1335651473-19652-1-git-send-email-amdragon@mit.edu>
	<1335803138-24024-1-git-send-email-amdragon@mit.edu>
Precedence: list

It has been a long-standing issue that notmuch_database_open doesn't
return any indication of why it failed.  This patch changes its
prototype to return a notmuch_status_t and set an out-argument to the
database itself, like other functions that return both a status and an
object.

In the interest of atomicity, this also updates every use in the CLI
so that notmuch still compiles.  Since this patch does not update the
bindings, the Python bindings test fails.
---
 lib/database.cc     |   28 +++++++++++++++++++++++-----
 lib/notmuch.h       |   26 +++++++++++++++++++-------
 notmuch-count.c     |    5 ++---
 notmuch-dump.c      |    5 ++---
 notmuch-new.c       |    5 ++---
 notmuch-reply.c     |    5 ++---
 notmuch-restore.c   |    5 ++---
 notmuch-search.c    |    5 ++---
 notmuch-show.c      |    5 ++---
 notmuch-tag.c       |    5 ++---
 test/symbol-test.cc |    3 ++-
 11 files changed, 60 insertions(+), 37 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index 2fefcad..1e66599 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -556,8 +556,9 @@ notmuch_database_create (const char *path)
 	goto DONE;
     }
 
-    notmuch = notmuch_database_open (path,
-				     NOTMUCH_DATABASE_MODE_READ_WRITE);
+    notmuch_database_open (path,
+			   NOTMUCH_DATABASE_MODE_READ_WRITE,
+			   &notmuch);
     notmuch_database_upgrade (notmuch, NULL, NULL);
 
   DONE:
@@ -578,10 +579,12 @@ _notmuch_database_ensure_writable (notmuch_database_t *notmuch)
     return NOTMUCH_STATUS_SUCCESS;
 }
 
-notmuch_database_t *
+notmuch_status_t
 notmuch_database_open (const char *path,
-		       notmuch_database_mode_t mode)
+		       notmuch_database_mode_t mode,
+		       notmuch_database_t **database)
 {
+    notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
     void *local = talloc_new (NULL);
     notmuch_database_t *notmuch = NULL;
     char *notmuch_path, *xapian_path;
@@ -590,8 +593,15 @@ notmuch_database_open (const char *path,
     unsigned int i, version;
     static int initialized = 0;
 
+    if (path == NULL) {
+	fprintf (stderr, "Error: Cannot open a database for a NULL path.\n");
+	status = NOTMUCH_STATUS_NULL_POINTER;
+	goto DONE;
+    }
+
     if (! (notmuch_path = talloc_asprintf (local, "%s/%s", path, ".notmuch"))) {
 	fprintf (stderr, "Out of memory\n");
+	status = NOTMUCH_STATUS_OUT_OF_MEMORY;
 	goto DONE;
     }
 
@@ -599,11 +609,13 @@ notmuch_database_open (const char *path,
     if (err) {
 	fprintf (stderr, "Error opening database at %s: %s\n",
 		 notmuch_path, strerror (errno));
+	status = NOTMUCH_STATUS_FILE_ERROR;
 	goto DONE;
     }
 
     if (! (xapian_path = talloc_asprintf (local, "%s/%s", notmuch_path, "xapian"))) {
 	fprintf (stderr, "Out of memory\n");
+	status = NOTMUCH_STATUS_OUT_OF_MEMORY;
 	goto DONE;
     }
 
@@ -644,6 +656,7 @@ notmuch_database_open (const char *path,
 		notmuch->mode = NOTMUCH_DATABASE_MODE_READ_ONLY;
 		notmuch_database_destroy (notmuch);
 		notmuch = NULL;
+		status = NOTMUCH_STATUS_FILE_ERROR;
 		goto DONE;
 	    }
 
@@ -704,12 +717,17 @@ notmuch_database_open (const char *path,
 		 error.get_msg().c_str());
 	notmuch_database_destroy (notmuch);
 	notmuch = NULL;
+	status = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
     }
 
   DONE:
     talloc_free (local);
 
-    return notmuch;
+    if (database)
+	*database = notmuch;
+    else
+	talloc_free (notmuch);
+    return status;
 }
 
 void
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 7d9e092..44b0c46 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -151,9 +151,6 @@ typedef enum {
     NOTMUCH_DATABASE_MODE_READ_WRITE
 } notmuch_database_mode_t;
 
-/* XXX: I think I'd like this to take an extra argument of
- * notmuch_status_t* for returning a status value on failure. */
-
 /* Open an existing notmuch database located at 'path'.
  *
  * The database should have been created at some time in the past,
@@ -168,12 +165,27 @@ typedef enum {
  * The caller should call notmuch_database_destroy when finished with
  * this database.
  *
- * In case of any failure, this function returns NULL, (after printing
- * an error message on stderr).
+ * In case of any failure, this function returns an error status and
+ * sets *database to NULL (after printing an error message on stderr).
+ *
+ * Return value:
+ *
+ * NOTMUCH_STATUS_SUCCESS: Successfully opened the database.
+ *
+ * NOTMUCH_STATUS_NULL_POINTER: The given 'path' argument is NULL.
+ *
+ * NOTMUCH_STATUS_OUT_OF_MEMORY: Out of memory.
+ *
+ * NOTMUCH_STATUS_FILE_ERROR: An error occurred trying to open the
+ *	database file (such as permission denied, or file not found,
+ *	etc.), or the database version is unknown.
+ *
+ * NOTMUCH_STATUS_XAPIAN_EXCEPTION: A Xapian exception occurred.
  */
-notmuch_database_t *
+notmuch_status_t
 notmuch_database_open (const char *path,
-		       notmuch_database_mode_t mode);
+		       notmuch_database_mode_t mode,
+		       notmuch_database_t **database);
 
 /* Close the given notmuch database.
  *
diff --git a/notmuch-count.c b/notmuch-count.c
index 9c2ad7b..2f98128 100644
--- a/notmuch-count.c
+++ b/notmuch-count.c
@@ -66,9 +66,8 @@ notmuch_count_command (void *ctx, int argc, char *argv[])
     if (config == NULL)
 	return 1;
 
-    notmuch = notmuch_database_open (notmuch_config_get_database_path (config),
-				     NOTMUCH_DATABASE_MODE_READ_ONLY);
-    if (notmuch == NULL)
+    if (notmuch_database_open (notmuch_config_get_database_path (config),
+			       NOTMUCH_DATABASE_MODE_READ_ONLY, &notmuch))
 	return 1;
 
     query_str = query_string_from_args (ctx, argc-opt_index, argv+opt_index);
diff --git a/notmuch-dump.c b/notmuch-dump.c
index 71ab0ea..3743214 100644
--- a/notmuch-dump.c
+++ b/notmuch-dump.c
@@ -36,9 +36,8 @@ notmuch_dump_command (unused (void *ctx), int argc, char *argv[])
     if (config == NULL)
 	return 1;
 
-    notmuch = notmuch_database_open (notmuch_config_get_database_path (config),
-				     NOTMUCH_DATABASE_MODE_READ_ONLY);
-    if (notmuch == NULL)
+    if (notmuch_database_open (notmuch_config_get_database_path (config),
+			       NOTMUCH_DATABASE_MODE_READ_ONLY, &notmuch))
 	return 1;
 
     char *output_file_name = NULL;
diff --git a/notmuch-new.c b/notmuch-new.c
index 3ff6304..7788743 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -903,9 +903,8 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
 	notmuch = notmuch_database_create (db_path);
 	add_files_state.total_files = count;
     } else {
-	notmuch = notmuch_database_open (db_path,
-					 NOTMUCH_DATABASE_MODE_READ_WRITE);
-	if (notmuch == NULL)
+	if (notmuch_database_open (db_path, NOTMUCH_DATABASE_MODE_READ_WRITE,
+				   &notmuch))
 	    return 1;
 
 	if (notmuch_database_needs_upgrade (notmuch)) {
diff --git a/notmuch-reply.c b/notmuch-reply.c
index da99a13..7184a5d 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -740,9 +740,8 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
 	return 1;
     }
 
-    notmuch = notmuch_database_open (notmuch_config_get_database_path (config),
-				     NOTMUCH_DATABASE_MODE_READ_ONLY);
-    if (notmuch == NULL)
+    if (notmuch_database_open (notmuch_config_get_database_path (config),
+			       NOTMUCH_DATABASE_MODE_READ_ONLY, &notmuch))
 	return 1;
 
     query = notmuch_query_create (notmuch, query_string);
diff --git a/notmuch-restore.c b/notmuch-restore.c
index 02b563c..4f4096e 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -115,9 +115,8 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[])
     if (config == NULL)
 	return 1;
 
-    notmuch = notmuch_database_open (notmuch_config_get_database_path (config),
-				     NOTMUCH_DATABASE_MODE_READ_WRITE);
-    if (notmuch == NULL)
+    if (notmuch_database_open (notmuch_config_get_database_path (config),
+			       NOTMUCH_DATABASE_MODE_READ_WRITE, &notmuch))
 	return 1;
 
     synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config);
diff --git a/notmuch-search.c b/notmuch-search.c
index 7dfd270..3be296d 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -486,9 +486,8 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
     if (config == NULL)
 	return 1;
 
-    notmuch = notmuch_database_open (notmuch_config_get_database_path (config),
-				     NOTMUCH_DATABASE_MODE_READ_ONLY);
-    if (notmuch == NULL)
+    if (notmuch_database_open (notmuch_config_get_database_path (config),
+			       NOTMUCH_DATABASE_MODE_READ_ONLY, &notmuch))
 	return 1;
 
     query_str = query_string_from_args (notmuch, argc-opt_index, argv+opt_index);
diff --git a/notmuch-show.c b/notmuch-show.c
index 3b6667c..95427d4 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -1081,9 +1081,8 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
 	return 1;
     }
 
-    notmuch = notmuch_database_open (notmuch_config_get_database_path (config),
-				     NOTMUCH_DATABASE_MODE_READ_ONLY);
-    if (notmuch == NULL)
+    if (notmuch_database_open (notmuch_config_get_database_path (config),
+			       NOTMUCH_DATABASE_MODE_READ_ONLY, &notmuch))
 	return 1;
 
     query = notmuch_query_create (notmuch, query_string);
diff --git a/notmuch-tag.c b/notmuch-tag.c
index bd56fd1..7d18639 100644
--- a/notmuch-tag.c
+++ b/notmuch-tag.c
@@ -229,9 +229,8 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
     if (config == NULL)
 	return 1;
 
-    notmuch = notmuch_database_open (notmuch_config_get_database_path (config),
-				     NOTMUCH_DATABASE_MODE_READ_WRITE);
-    if (notmuch == NULL)
+    if (notmuch_database_open (notmuch_config_get_database_path (config),
+			       NOTMUCH_DATABASE_MODE_READ_WRITE, &notmuch))
 	return 1;
 
     synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config);
diff --git a/test/symbol-test.cc b/test/symbol-test.cc
index 1548ca4..3e96c03 100644
--- a/test/symbol-test.cc
+++ b/test/symbol-test.cc
@@ -4,7 +4,8 @@
 
 
 int main() {
-  (void) notmuch_database_open("fakedb", NOTMUCH_DATABASE_MODE_READ_ONLY);
+  notmuch_database_t *notmuch;
+  notmuch_database_open("fakedb", NOTMUCH_DATABASE_MODE_READ_ONLY, &notmuch);
 
   try {
     (void) new Xapian::WritableDatabase("./nonexistant", Xapian::DB_OPEN);
-- 
1.7.9.1