[PATCH 0/6] Make notmuch_database_{open,create} return status codes

[PATCH 0/6] Make notmuch_database_{open,create} return status codes

[Austin Clements]

Since we're breaking binary and source compatibility with the next
release anyway, it's about time we fix notmuch_database_{open,create}
to return status codes.

[PATCH 1/6] lib/cli: Make notmuch_database_open return a status code

[Austin Clements]

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     |   19 ++++++++++++++-----
 lib/notmuch.h       |   12 +++++-------
 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, 37 insertions(+), 37 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index 2fefcad..a29fe67 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;
@@ -592,6 +595,7 @@ notmuch_database_open (const char *path,
 
     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 +603,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 +650,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 +711,14 @@ 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;
+    *database = notmuch;
+    return status;
 }
 
 void
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 7d9e092..8a011f2 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,13 @@ 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).
  */
-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

[PATCH 2/6] lib/cli: Make notmuch_database_create return a status code

[Austin Clements]

This is the notmuch_database_create equivalent of the previous change.

In this case, there were places where errors were not being propagated
correctly in notmuch_database_create or in calls to it.  These have
been fixed, using the new status value.
---
 lib/database.cc |   26 +++++++++++++++++++-------
 lib/notmuch.h   |    8 ++++----
 notmuch-new.c   |    3 ++-
 3 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index a29fe67..a3ae504 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -520,9 +520,10 @@ parse_references (void *ctx,
     }
 }
 
-notmuch_database_t *
-notmuch_database_create (const char *path)
+notmuch_status_t
+notmuch_database_create (const char *path, notmuch_database_t **database)
 {
+    notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
     notmuch_database_t *notmuch = NULL;
     char *notmuch_path = NULL;
     struct stat st;
@@ -530,6 +531,7 @@ notmuch_database_create (const char *path)
 
     if (path == NULL) {
 	fprintf (stderr, "Error: Cannot create a database for a NULL path.\n");
+	status = NOTMUCH_STATUS_NULL_POINTER;
 	goto DONE;
     }
 
@@ -537,12 +539,14 @@ notmuch_database_create (const char *path)
     if (err) {
 	fprintf (stderr, "Error: Cannot create database at %s: %s.\n",
 		 path, strerror (errno));
+	status = NOTMUCH_STATUS_FILE_ERROR;
 	goto DONE;
     }
 
     if (! S_ISDIR (st.st_mode)) {
 	fprintf (stderr, "Error: Cannot create database at %s: Not a directory.\n",
 		 path);
+	status = NOTMUCH_STATUS_FILE_ERROR;
 	goto DONE;
     }
 
@@ -553,19 +557,27 @@ notmuch_database_create (const char *path)
     if (err) {
 	fprintf (stderr, "Error: Cannot create directory %s: %s.\n",
 		 notmuch_path, strerror (errno));
+	status = NOTMUCH_STATUS_FILE_ERROR;
 	goto DONE;
     }
 
-    notmuch_database_open (path,
-			   NOTMUCH_DATABASE_MODE_READ_WRITE,
-			   &notmuch);
-    notmuch_database_upgrade (notmuch, NULL, NULL);
+    status = notmuch_database_open (path,
+				    NOTMUCH_DATABASE_MODE_READ_WRITE,
+				    &notmuch);
+    if (status)
+	goto DONE;
+    status = notmuch_database_upgrade (notmuch, NULL, NULL);
+    if (status) {
+	notmuch_database_close(notmuch);
+	notmuch = NULL;
+    }
 
   DONE:
     if (notmuch_path)
 	talloc_free (notmuch_path);
 
-    return notmuch;
+    *database = notmuch;
+    return status;
 }
 
 notmuch_status_t
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 8a011f2..d880aeb 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -140,11 +140,11 @@ typedef struct _notmuch_filenames notmuch_filenames_t;
  * contained within 'path' can be added to the database by calling
  * notmuch_database_add_message.
  *
- * 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).
  */
-notmuch_database_t *
-notmuch_database_create (const char *path);
+notmuch_status_t
+notmuch_database_create (const char *path, notmuch_database_t **database);
 
 typedef enum {
     NOTMUCH_DATABASE_MODE_READ_ONLY = 0,
diff --git a/notmuch-new.c b/notmuch-new.c
index 7788743..cb720cc 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -900,7 +900,8 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
 	    return 1;
 
 	printf ("Found %d total files (that's not much mail).\n", count);
-	notmuch = notmuch_database_create (db_path);
+	if (notmuch_database_create (db_path, &notmuch))
+	    return 1;
 	add_files_state.total_files = count;
     } else {
 	if (notmuch_database_open (db_path, NOTMUCH_DATABASE_MODE_READ_WRITE,
-- 
1.7.9.1

[PATCH 3/6] go: Update Go bindings for new notmuch_database_{open, create} signatures

[Austin Clements]

This requires changing the return types of NewDatabase and
OpenDatabase to follow the standard Go convention for returning
errors.
---
 bindings/go/pkg/notmuch.go |   24 ++++++++++++------------
 1 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/bindings/go/pkg/notmuch.go b/bindings/go/pkg/notmuch.go
index d32901d..86e9645 100644
--- a/bindings/go/pkg/notmuch.go
+++ b/bindings/go/pkg/notmuch.go
@@ -86,21 +86,21 @@ const (
 )
 
 // Create a new, empty notmuch database located at 'path'
-func NewDatabase(path string) *Database {
+func NewDatabase(path string) (*Database, Status) {
 
 	var c_path *C.char = C.CString(path)
 	defer C.free(unsafe.Pointer(c_path))
 
 	if c_path == nil {
-		return nil
+		return nil, STATUS_OUT_OF_MEMORY
 	}
 
 	self := &Database{db:nil}
-	self.db = C.notmuch_database_create(c_path)
-	if self.db == nil {
-		return nil
+	st := Status(C.notmuch_database_create(c_path, &self.db))
+	if st != STATUS_SUCCESS {
+		return nil, st
 	}
-	return self
+	return self, st
 }
 
 /* Open an existing notmuch database located at 'path'.
@@ -120,21 +120,21 @@ func NewDatabase(path string) *Database {
  * In case of any failure, this function returns NULL, (after printing
  * an error message on stderr).
  */
-func OpenDatabase(path string, mode DatabaseMode) *Database {
+func OpenDatabase(path string, mode DatabaseMode) (*Database, Status) {
 
 	var c_path *C.char = C.CString(path)
 	defer C.free(unsafe.Pointer(c_path))
 
 	if c_path == nil {
-		return nil
+		return nil, STATUS_OUT_OF_MEMORY
 	}
 
 	self := &Database{db:nil}
-	self.db = C.notmuch_database_open(c_path, C.notmuch_database_mode_t(mode))
-	if self.db == nil {
-		return nil
+	st := Status(C.notmuch_database_open(c_path, C.notmuch_database_mode_t(mode), &self.db))
+	if st != STATUS_SUCCESS {
+		return nil, st
 	}
-	return self
+	return self, st
 }
 
 /* Close the given notmuch database, freeing all associated
-- 
1.7.9.1

[PATCH 4/6] python: Update Python bindings for new notmuch_database_{open, create} signatures

[Austin Clements]

---
 bindings/python/notmuch/database.py |   29 ++++++++++++++++-------------
 1 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/bindings/python/notmuch/database.py b/bindings/python/notmuch/database.py
index 268e952..adc0a3f 100644
--- a/bindings/python/notmuch/database.py
+++ b/bindings/python/notmuch/database.py
@@ -95,8 +95,8 @@ class Database(object):
 
     """notmuch_database_open"""
     _open = nmlib.notmuch_database_open
-    _open.argtypes = [c_char_p, c_uint]
-    _open.restype = NotmuchDatabaseP
+    _open.argtypes = [c_char_p, c_uint, POINTER(NotmuchDatabaseP)]
+    _open.restype = c_uint
 
     """notmuch_database_upgrade"""
     _upgrade = nmlib.notmuch_database_upgrade
@@ -122,8 +122,8 @@ class Database(object):
 
     """notmuch_database_create"""
     _create = nmlib.notmuch_database_create
-    _create.argtypes = [c_char_p]
-    _create.restype = NotmuchDatabaseP
+    _create.argtypes = [c_char_p, POINTER(NotmuchDatabaseP)]
+    _create.restype = c_uint
 
     def __init__(self, path = None, create = False,
                  mode = MODE.READ_ONLY):
@@ -193,12 +193,13 @@ class Database(object):
             raise NotmuchError(message="Cannot create db, this Database() "
                                        "already has an open one.")
 
-        res = Database._create(_str(path), Database.MODE.READ_WRITE)
+        db = NotmuchDatabaseP()
+        status = Database._create(_str(path), Database.MODE.READ_WRITE, byref(db))
 
-        if not res:
-            raise NotmuchError(
-                message="Could not create the specified database")
-        self._db = res
+        if status != STATUS.SUCCESS:
+            raise NotmuchError(status)
+        self._db = db
+        return status
 
     def open(self, path, mode=0):
         """Opens an existing database
@@ -212,11 +213,13 @@ class Database(object):
         :raises: Raises :exc:`NotmuchError` in case of any failure
                     (possibly after printing an error message on stderr).
         """
-        res = Database._open(_str(path), mode)
+        db = NotmuchDatabaseP()
+        status = Database._open(_str(path), mode, byref(db))
 
-        if not res:
-            raise NotmuchError(message="Could not open the specified database")
-        self._db = res
+        if status != STATUS.SUCCESS:
+            raise NotmuchError(status)
+        self._db = db
+        return status
 
     _close = nmlib.notmuch_database_close
     _close.argtypes = [NotmuchDatabaseP]
-- 
1.7.9.1

[PATCH 5/6] ruby: Update Ruby bindings for new notmuch_database_{open, create} signatures

[Austin Clements]

---
 bindings/ruby/database.c |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/bindings/ruby/database.c b/bindings/ruby/database.c
index ba9a139..409d54f 100644
--- a/bindings/ruby/database.c
+++ b/bindings/ruby/database.c
@@ -42,6 +42,8 @@ notmuch_rb_database_initialize (int argc, VALUE *argv, VALUE self)
     int create, mode;
     VALUE pathv, hashv;
     VALUE modev;
+    notmuch_database_t *database;
+    notmuch_status_t ret;
 
     /* Check arguments */
     rb_scan_args (argc, argv, "11", &pathv, &hashv);
@@ -73,9 +75,13 @@ notmuch_rb_database_initialize (int argc, VALUE *argv, VALUE self)
     }
 
     Check_Type (self, T_DATA);
-    DATA_PTR (self) = create ? notmuch_database_create (path) : notmuch_database_open (path, mode);
-    if (!DATA_PTR (self))
-	rb_raise (notmuch_rb_eDatabaseError, "Failed to open database");
+    if (create)
+	ret = notmuch_database_create (path, &database);
+    else
+	ret = notmuch_database_open (path, mode, &database);
+    notmuch_rb_status_raise (ret);
+
+    DATA_PTR (self) = database;
 
     return self;
 }
-- 
1.7.9.1

[PATCH 6/6] News for changes to notmuch_database_{open,create}

[Austin Clements]

---
 NEWS |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/NEWS b/NEWS
index a2cd080..0f031f3 100644
--- a/NEWS
+++ b/NEWS
@@ -76,15 +76,20 @@ contrib/ from now on.
 Library changes
 ---------------
 
-API changes
-
-  The function notmuch_database_close has been split into
-  notmuch_database_close and notmuch_database_destroy.
+The function notmuch_database_close has been split into
+notmuch_database_close and notmuch_database_destroy
 
   This makes it possible for long running programs to close the xapian
   database and thus release the lock associated with it without
   destroying the data structures obtained from it.
 
+notmuch_database_open and notmuch_database_create now return errors
+
+  The type signatures of notmuch_database_open and
+  notmuch_database_create have changed so that the functions now
+  return a notmuch_status_t and take an out-argument for returning the
+  new database object.
+
 Notmuch 0.12 (2012-03-20)
 =========================
 
-- 
1.7.9.1

Re: [PATCH 0/6] Make notmuch_database_{open, create} return status codes

[Justus Winter]

Hi Austin :)

Quoting Austin Clements (2012-04-29 00:17:47)
> Since we're breaking binary and source compatibility with the next
> release anyway, it's about time we fix notmuch_database_{open,create}
> to return status codes.

Awesome, thanks for taking care of this. The python patch looks fine.

Justus

Re: [PATCH 1/6] lib/cli: Make notmuch_database_open return a status code

[David Bremner]

I have two questions, both of which might have been settled previously.

>  
> -    return notmuch;
> +    *database = notmuch;
> +    return status;
>  }

Should we check database is non-NULL here?

> diff --git a/notmuch-count.c b/notmuch-count.c
> index 9c2ad7b..2f98128 100644
> --- a/notmuch-count.c
> +++ b/notmuch-count.c

> +    if (notmuch_database_open (notmuch_config_get_database_path (config),
> +			       NOTMUCH_DATABASE_MODE_READ_ONLY, &notmuch))

Is it OK to rely on NOTMUCH_STATUS_SUCCESS==0 ?

d

Re: [PATCH 1/6] lib/cli: Make notmuch_database_open return a status code

[Austin Clements]

Quoth David Bremner on Apr 30 at  8:39 am:
> 
> I have two questions, both of which might have been settled previously.
> 
> >  
> > -    return notmuch;
> > +    *database = notmuch;
> > +    return status;
> >  }
> 
> Should we check database is non-NULL here?

Sure, why not.  Since one could now imagine using these functions to
create a database without using it or test if a database exists, I
made them handle a NULL pointer gracefully.

> > diff --git a/notmuch-count.c b/notmuch-count.c
> > index 9c2ad7b..2f98128 100644
> > --- a/notmuch-count.c
> > +++ b/notmuch-count.c
> 
> > +    if (notmuch_database_open (notmuch_config_get_database_path (config),
> > +			       NOTMUCH_DATABASE_MODE_READ_ONLY, &notmuch))
> 
> Is it OK to rely on NOTMUCH_STATUS_SUCCESS==0 ?

I decree it so.  Plus, we already do this sort of thing all over the
place.

[PATCH v2 0/6] Make notmuch_database_{open, create} return status codes

[Austin Clements]

Relative to v1, this makes notmuch_database_open and
notmuch_database_create gracefully handle a NULL out-argument and adds
documentation of the possible error return values from these two
functions.  Patches 3 and on have not changed.

[PATCH v2 1/6] lib/cli: Make notmuch_database_open return a status code

[Austin Clements]

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

[PATCH v2 2/6] lib/cli: Make notmuch_database_create return a status code

[Austin Clements]

This is the notmuch_database_create equivalent of the previous change.

In this case, there were places where errors were not being propagated
correctly in notmuch_database_create or in calls to it.  These have
been fixed, using the new status value.
---
 lib/database.cc |   29 ++++++++++++++++++++++-------
 lib/notmuch.h   |   22 ++++++++++++++++++----
 notmuch-new.c   |    3 ++-
 3 files changed, 42 insertions(+), 12 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index 1e66599..07f186e 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -520,9 +520,10 @@ parse_references (void *ctx,
     }
 }
 
-notmuch_database_t *
-notmuch_database_create (const char *path)
+notmuch_status_t
+notmuch_database_create (const char *path, notmuch_database_t **database)
 {
+    notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
     notmuch_database_t *notmuch = NULL;
     char *notmuch_path = NULL;
     struct stat st;
@@ -530,6 +531,7 @@ notmuch_database_create (const char *path)
 
     if (path == NULL) {
 	fprintf (stderr, "Error: Cannot create a database for a NULL path.\n");
+	status = NOTMUCH_STATUS_NULL_POINTER;
 	goto DONE;
     }
 
@@ -537,12 +539,14 @@ notmuch_database_create (const char *path)
     if (err) {
 	fprintf (stderr, "Error: Cannot create database at %s: %s.\n",
 		 path, strerror (errno));
+	status = NOTMUCH_STATUS_FILE_ERROR;
 	goto DONE;
     }
 
     if (! S_ISDIR (st.st_mode)) {
 	fprintf (stderr, "Error: Cannot create database at %s: Not a directory.\n",
 		 path);
+	status = NOTMUCH_STATUS_FILE_ERROR;
 	goto DONE;
     }
 
@@ -553,19 +557,30 @@ notmuch_database_create (const char *path)
     if (err) {
 	fprintf (stderr, "Error: Cannot create directory %s: %s.\n",
 		 notmuch_path, strerror (errno));
+	status = NOTMUCH_STATUS_FILE_ERROR;
 	goto DONE;
     }
 
-    notmuch_database_open (path,
-			   NOTMUCH_DATABASE_MODE_READ_WRITE,
-			   &notmuch);
-    notmuch_database_upgrade (notmuch, NULL, NULL);
+    status = notmuch_database_open (path,
+				    NOTMUCH_DATABASE_MODE_READ_WRITE,
+				    &notmuch);
+    if (status)
+	goto DONE;
+    status = notmuch_database_upgrade (notmuch, NULL, NULL);
+    if (status) {
+	notmuch_database_close(notmuch);
+	notmuch = NULL;
+    }
 
   DONE:
     if (notmuch_path)
 	talloc_free (notmuch_path);
 
-    return notmuch;
+    if (database)
+	*database = notmuch;
+    else
+	talloc_free (notmuch);
+    return status;
 }
 
 notmuch_status_t
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 44b0c46..e6e5cc2 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -140,11 +140,25 @@ typedef struct _notmuch_filenames notmuch_filenames_t;
  * contained within 'path' can be added to the database by calling
  * notmuch_database_add_message.
  *
- * 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 created 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 create the
+ *	database file (such as permission denied, or file not found,
+ *	etc.), or the database already exists.
+ *
+ * NOTMUCH_STATUS_XAPIAN_EXCEPTION: A Xapian exception occurred.
  */
-notmuch_database_t *
-notmuch_database_create (const char *path);
+notmuch_status_t
+notmuch_database_create (const char *path, notmuch_database_t **database);
 
 typedef enum {
     NOTMUCH_DATABASE_MODE_READ_ONLY = 0,
diff --git a/notmuch-new.c b/notmuch-new.c
index 7788743..cb720cc 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -900,7 +900,8 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
 	    return 1;
 
 	printf ("Found %d total files (that's not much mail).\n", count);
-	notmuch = notmuch_database_create (db_path);
+	if (notmuch_database_create (db_path, &notmuch))
+	    return 1;
 	add_files_state.total_files = count;
     } else {
 	if (notmuch_database_open (db_path, NOTMUCH_DATABASE_MODE_READ_WRITE,
-- 
1.7.9.1

[PATCH v2 3/6] go: Update Go bindings for new notmuch_database_{open, create} signatures

[Austin Clements]

This requires changing the return types of NewDatabase and
OpenDatabase to follow the standard Go convention for returning
errors.
---
 bindings/go/pkg/notmuch.go |   24 ++++++++++++------------
 1 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/bindings/go/pkg/notmuch.go b/bindings/go/pkg/notmuch.go
index d32901d..86e9645 100644
--- a/bindings/go/pkg/notmuch.go
+++ b/bindings/go/pkg/notmuch.go
@@ -86,21 +86,21 @@ const (
 )
 
 // Create a new, empty notmuch database located at 'path'
-func NewDatabase(path string) *Database {
+func NewDatabase(path string) (*Database, Status) {
 
 	var c_path *C.char = C.CString(path)
 	defer C.free(unsafe.Pointer(c_path))
 
 	if c_path == nil {
-		return nil
+		return nil, STATUS_OUT_OF_MEMORY
 	}
 
 	self := &Database{db:nil}
-	self.db = C.notmuch_database_create(c_path)
-	if self.db == nil {
-		return nil
+	st := Status(C.notmuch_database_create(c_path, &self.db))
+	if st != STATUS_SUCCESS {
+		return nil, st
 	}
-	return self
+	return self, st
 }
 
 /* Open an existing notmuch database located at 'path'.
@@ -120,21 +120,21 @@ func NewDatabase(path string) *Database {
  * In case of any failure, this function returns NULL, (after printing
  * an error message on stderr).
  */
-func OpenDatabase(path string, mode DatabaseMode) *Database {
+func OpenDatabase(path string, mode DatabaseMode) (*Database, Status) {
 
 	var c_path *C.char = C.CString(path)
 	defer C.free(unsafe.Pointer(c_path))
 
 	if c_path == nil {
-		return nil
+		return nil, STATUS_OUT_OF_MEMORY
 	}
 
 	self := &Database{db:nil}
-	self.db = C.notmuch_database_open(c_path, C.notmuch_database_mode_t(mode))
-	if self.db == nil {
-		return nil
+	st := Status(C.notmuch_database_open(c_path, C.notmuch_database_mode_t(mode), &self.db))
+	if st != STATUS_SUCCESS {
+		return nil, st
 	}
-	return self
+	return self, st
 }
 
 /* Close the given notmuch database, freeing all associated
-- 
1.7.9.1

[PATCH v2 4/6] python: Update Python bindings for new notmuch_database_{open, create} signatures

[Austin Clements]

---
 bindings/python/notmuch/database.py |   29 ++++++++++++++++-------------
 1 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/bindings/python/notmuch/database.py b/bindings/python/notmuch/database.py
index 268e952..adc0a3f 100644
--- a/bindings/python/notmuch/database.py
+++ b/bindings/python/notmuch/database.py
@@ -95,8 +95,8 @@ class Database(object):
 
     """notmuch_database_open"""
     _open = nmlib.notmuch_database_open
-    _open.argtypes = [c_char_p, c_uint]
-    _open.restype = NotmuchDatabaseP
+    _open.argtypes = [c_char_p, c_uint, POINTER(NotmuchDatabaseP)]
+    _open.restype = c_uint
 
     """notmuch_database_upgrade"""
     _upgrade = nmlib.notmuch_database_upgrade
@@ -122,8 +122,8 @@ class Database(object):
 
     """notmuch_database_create"""
     _create = nmlib.notmuch_database_create
-    _create.argtypes = [c_char_p]
-    _create.restype = NotmuchDatabaseP
+    _create.argtypes = [c_char_p, POINTER(NotmuchDatabaseP)]
+    _create.restype = c_uint
 
     def __init__(self, path = None, create = False,
                  mode = MODE.READ_ONLY):
@@ -193,12 +193,13 @@ class Database(object):
             raise NotmuchError(message="Cannot create db, this Database() "
                                        "already has an open one.")
 
-        res = Database._create(_str(path), Database.MODE.READ_WRITE)
+        db = NotmuchDatabaseP()
+        status = Database._create(_str(path), Database.MODE.READ_WRITE, byref(db))
 
-        if not res:
-            raise NotmuchError(
-                message="Could not create the specified database")
-        self._db = res
+        if status != STATUS.SUCCESS:
+            raise NotmuchError(status)
+        self._db = db
+        return status
 
     def open(self, path, mode=0):
         """Opens an existing database
@@ -212,11 +213,13 @@ class Database(object):
         :raises: Raises :exc:`NotmuchError` in case of any failure
                     (possibly after printing an error message on stderr).
         """
-        res = Database._open(_str(path), mode)
+        db = NotmuchDatabaseP()
+        status = Database._open(_str(path), mode, byref(db))
 
-        if not res:
-            raise NotmuchError(message="Could not open the specified database")
-        self._db = res
+        if status != STATUS.SUCCESS:
+            raise NotmuchError(status)
+        self._db = db
+        return status
 
     _close = nmlib.notmuch_database_close
     _close.argtypes = [NotmuchDatabaseP]
-- 
1.7.9.1

[PATCH v2 5/6] ruby: Update Ruby bindings for new notmuch_database_{open, create} signatures

[Austin Clements]

---
 bindings/ruby/database.c |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/bindings/ruby/database.c b/bindings/ruby/database.c
index ba9a139..409d54f 100644
--- a/bindings/ruby/database.c
+++ b/bindings/ruby/database.c
@@ -42,6 +42,8 @@ notmuch_rb_database_initialize (int argc, VALUE *argv, VALUE self)
     int create, mode;
     VALUE pathv, hashv;
     VALUE modev;
+    notmuch_database_t *database;
+    notmuch_status_t ret;
 
     /* Check arguments */
     rb_scan_args (argc, argv, "11", &pathv, &hashv);
@@ -73,9 +75,13 @@ notmuch_rb_database_initialize (int argc, VALUE *argv, VALUE self)
     }
 
     Check_Type (self, T_DATA);
-    DATA_PTR (self) = create ? notmuch_database_create (path) : notmuch_database_open (path, mode);
-    if (!DATA_PTR (self))
-	rb_raise (notmuch_rb_eDatabaseError, "Failed to open database");
+    if (create)
+	ret = notmuch_database_create (path, &database);
+    else
+	ret = notmuch_database_open (path, mode, &database);
+    notmuch_rb_status_raise (ret);
+
+    DATA_PTR (self) = database;
 
     return self;
 }
-- 
1.7.9.1

[PATCH v2 6/6] News for changes to notmuch_database_{open,create}

[Austin Clements]

---
 NEWS |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/NEWS b/NEWS
index a2cd080..0f031f3 100644
--- a/NEWS
+++ b/NEWS
@@ -76,15 +76,20 @@ contrib/ from now on.
 Library changes
 ---------------
 
-API changes
-
-  The function notmuch_database_close has been split into
-  notmuch_database_close and notmuch_database_destroy.
+The function notmuch_database_close has been split into
+notmuch_database_close and notmuch_database_destroy
 
   This makes it possible for long running programs to close the xapian
   database and thus release the lock associated with it without
   destroying the data structures obtained from it.
 
+notmuch_database_open and notmuch_database_create now return errors
+
+  The type signatures of notmuch_database_open and
+  notmuch_database_create have changed so that the functions now
+  return a notmuch_status_t and take an out-argument for returning the
+  new database object.
+
 Notmuch 0.12 (2012-03-20)
 =========================
 
-- 
1.7.9.1

Re: [PATCH v2 0/6] Make notmuch_database_{open, create} return status codes

[Tomi Ollila]

On Mon, Apr 30 2012, Austin Clements <amdragon@MIT.EDU> wrote:

> Relative to v1, this makes notmuch_database_open and
> notmuch_database_create gracefully handle a NULL out-argument and adds
> documentation of the possible error return values from these two
> functions.  Patches 3 and on have not changed.

LGTM. 

One question though:

In bindings/python/notmuch/database.py class Database functions
create() and open() have the following last lines:

+        if status != STATUS.SUCCESS:
+            raise NotmuchError(status)
+        self._db = db
+        return status

What is the point returning 'status' in the only case the
value is STATUS.SUCCESS -- is some caller interested on this ?

Tomi

Re: [PATCH v2 0/6] Make notmuch_database_{open, create} return status codes

[Austin Clements]

Quoth Tomi Ollila on May 02 at 10:08 pm:
> On Mon, Apr 30 2012, Austin Clements <amdragon@MIT.EDU> wrote:
> 
> > Relative to v1, this makes notmuch_database_open and
> > notmuch_database_create gracefully handle a NULL out-argument and adds
> > documentation of the possible error return values from these two
> > functions.  Patches 3 and on have not changed.
> 
> LGTM. 
> 
> One question though:
> 
> In bindings/python/notmuch/database.py class Database functions
> create() and open() have the following last lines:
> 
> +        if status != STATUS.SUCCESS:
> +            raise NotmuchError(status)
> +        self._db = db
> +        return status
> 
> What is the point returning 'status' in the only case the
> value is STATUS.SUCCESS -- is some caller interested on this ?

This was for consistency with other methods (e.g., begin_atomic also
only ever returns STATUS.SUCCESS).

Re: [PATCH v2 1/6] lib/cli: Make notmuch_database_open return a status code

[David Bremner]

Austin Clements <amdragon@MIT.EDU> writes:

> 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.

pushed.

d