lib/cli: Make notmuch_database_open return a status code

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.

diff --git a/lib/database.cc b/lib/database.cc
index 2fefcad7d163b5e9e40f9805a3d58b46a5704e37..1e66599292855138a91db954d21affc11a7ac7ad 100644 (file)
--- 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 7d9e0921f0c17e9f442f46e57dcc33e152c13329..44b0c4608c39302a3d79666a7b3180eed683cae7 100644 (file)
--- 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 9c2ad7b2a9226bb8ee616779d88cde69171ac704..2f9812821ebeb7cf2f18542541beaf9a8e37e4be 100644 (file)
--- 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 71ab0ea73d69f44de2505099c86f605e0d3cb33e..37432142761216e4a87494c3ac57bf79ad207c99 100644 (file)
--- 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 3ff630478c613848f342ddbdb42fa625484254d0..7788743da44dcfca5e8ce5ba1dce085025a79b5c 100644 (file)
--- 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 da99a1374688befd36a859807862a25d7043a825..7184a5dfcf887bb6fcb81bc8f1ee63171f7bb286 100644 (file)
--- 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 02b563caae2dcbd5d52d223cf3ef6db4c2739d88..4f4096ed15c79b11d06b97abfaa0be0b8fc2af5c 100644 (file)
--- 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 7dfd270e2019c651d6738a04e2cca7766a92bff7..3be296d8a3f67db106d139c882544f3746eb4d8d 100644 (file)
--- 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 3b6667c4bdbd5d411fc35dabbe40bce8bc74d19b..95427d4fd3096f294714b8a1b4fb069f64fc0503 100644 (file)
--- 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 bd56fd14a4699b74f5567f6ac0e3d4284a49600e..7d186399ba786149d607ca2b45837836a43af802 100644 (file)
--- 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 1548ca400a544f3ddb325ce02826de1531cf602d..3e96c034e13e94412285bcde6dd2d74e870993d2 100644 (file)
--- 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);