From: Jani Nikula <jani@nikula.org>
To: Austin Clements <amdragon@MIT.EDU>
Subject: Re: [PATCH 2/2] lib: introduce notmuch_database_new for initializing
	a database handle
In-Reply-To: <20131204231113.GD8854@mit.edu>
References: <cover.1385903109.git.jani@nikula.org>
	<fc7ecd990e55fcfba17de4d71e8823c98760f9ce.1385903109.git.jani@nikula.org>
	<20131204231113.GD8854@mit.edu>
User-Agent: Notmuch/0.16+145~gebbaa94 (http://notmuchmail.org) Emacs/24.3.1
	(x86_64-pc-linux-gnu)
Date: Thu, 05 Dec 2013 20:17:58 +0200
Message-ID: <87mwkf40x5.fsf@nikula.org>
MIME-Version: 1.0
Content-Type: text/plain
Cc: notmuch@notmuchmail.org
Precedence: list

On Thu, 05 Dec 2013, Austin Clements <amdragon@MIT.EDU> wrote:
> Quoth Jani Nikula on Dec 01 at  3:14 pm:
>> There is a need for setting options before opening a database, such as
>> setting a logging function to use instead of writing to stdout or
>> stderr. It would be possible to do this by adding new parameters to
>> notmuch_database_create and notmuch_database_open, but maintaining a
>> backwards compatible API and ABI when new options are added becomes
>> burdensome.
>> 
>> Instead, split the opaque database object creation from
>> notmuch_database_create and notmuch_database_open into a new
>> notmuch_database_new call, to allow operations on the handle before
>> create and open. This creates API and ABI breakage now, but allows
>> easier future extensions.
>> 
>> The notmuch_database_new call becomes a natural pair to the already
>> existing notmuch_database_destroy, and it should be possible to call
>> open/close multiple times using an initialized handle.
>
> A high-level comment about the API: Currently, an allocated
> notmuch_database_t has two "memory states", if you will: open and
> closed.  (I wish it didn't have any memory states, and was on the
> fence about this API for a while until I realized that the ship had
> already sailed.) 

Just to confirm, are you referring to the state between close and
destroy/open?

Btw what is the use case we have separate close and destroy for? To be
able to access the data cached in memory after the db has been closed?

> It's pretty clear how all of the notmuch APIs will
> behave in both states (modulo some bounded non-determinism in the
> closed state).  I think this patch introduces a new "pre-open" state,
> and I don't know how most of the notmuch APIs behave in that state.
> My guess is poorly.  If it's feasible, I'd much rather a fresh baked
> notmuch_database_t act like it's in the closed state, including that
> notmuch_database_{create,open} are well-defined as transitions from
> closed state to open state (even if the closed state was reached by
> calling notmuch_database_close).  Or, if we do have a "pre-open"
> state, it should at least be well-specified what that means
> (preferably the specification is *not* "most APIs segfault").

Agreed.

> Orthogonally -- and this may be a complete pipe dream of mine -- if we
> just had a way to return more detailed error information than a simple
> error code from notmuch_database_{create,open}, I think we wouldn't
> need any of this.  Everything that these functions currently log
> (modulo one warning) is error details, so if we could return the error
> details *with the error* or somehow make them accessible, we wouldn't
> need a logger at this point (or at several other points in the
> library).

Agreed. I tried to look into this earlier, but was hitting dead ends
*if* we want to keep reporting user friendly error status in
open/create. Obviously any concrete suggestions would be most welcome!

>> ---
>>  lib/database.cc      | 64 ++++++++++++++++++++++++++++------------------------
>>  lib/notmuch.h        | 52 ++++++++++++++++++++++++++++++++----------
>>  notmuch-compact.c    | 11 ++++++++-
>>  notmuch-count.c      | 10 ++++++--
>>  notmuch-dump.c       | 10 ++++++--
>>  notmuch-insert.c     | 10 ++++++--
>>  notmuch-new.c        | 14 +++++++-----
>>  notmuch-reply.c      | 10 ++++++--
>>  notmuch-restore.c    | 10 ++++++--
>>  notmuch-search.c     | 10 ++++++--
>>  notmuch-show.c       | 10 ++++++--
>>  notmuch-tag.c        | 10 ++++++--
>>  test/random-corpus.c | 10 ++++++--
>>  test/symbol-test.cc  |  3 ++-
>>  14 files changed, 166 insertions(+), 68 deletions(-)
>> 
>> diff --git a/lib/database.cc b/lib/database.cc
>> index 98e2c31..386b93a 100644
>> --- a/lib/database.cc
>> +++ b/lib/database.cc
>> @@ -539,10 +539,21 @@ parse_references (void *ctx,
>>  }
>>  
>>  notmuch_status_t
>> -notmuch_database_create (const char *path, notmuch_database_t **database)
>> +notmuch_database_new (notmuch_database_t **notmuch)
>
> The naming of this is unfortunate...  Other APIs use x_create to
> allocate objects (e.g., notmuch_query_create, several internal APIs).
> I would lean towards calling this function notmuch_database_create,
> but that leaves the question of what to call the other.  While we're
> breaking APIs, would it be completely crazy to merge open and create
> into one API with an extra mode to indicate creation (it can be its
> own mode because creation implies read/write)?  (Or, in UNIX
> tradition, we could call this function notmuch_database_create and the
> other notmuch_database_creat.)  notmuch_database_create is already
> just a shell around notmuch_database_open (we could keep it as a
> separate function, but just make it internal).

Agreed on the naming being unfortunate, though I'll dodge further
bikeshedding. ;) Merging open and create sounds all right, though it's a
minor detail in the bigger picture of this patch.

The comments below seemed valid too, thanks.

BR,
Jani.


>
>> +{
>> +    /* Note: try to avoid error conditions! No error printing! */
>> +
>> +    *notmuch = talloc_zero (NULL, notmuch_database_t);
>> +    if (! *notmuch)
>> +	return NOTMUCH_STATUS_OUT_OF_MEMORY;
>> +
>> +    return NOTMUCH_STATUS_SUCCESS;
>> +}
>> +
>> +notmuch_status_t
>> +notmuch_database_create (notmuch_database_t *notmuch, const char *path)
>>  {
>
> This should fail if passed a database that is already open.
>
>>      notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
>> -    notmuch_database_t *notmuch = NULL;
>>      char *notmuch_path = NULL;
>>      struct stat st;
>>      int err;
>> @@ -579,25 +590,18 @@ notmuch_database_create (const char *path, notmuch_database_t **database)
>>  	goto DONE;
>>      }
>>  
>> -    status = notmuch_database_open (path,
>> -				    NOTMUCH_DATABASE_MODE_READ_WRITE,
>> -				    &notmuch);
>> +    status = notmuch_database_open (notmuch, path,
>> +				    NOTMUCH_DATABASE_MODE_READ_WRITE);
>>      if (status)
>>  	goto DONE;
>>      status = notmuch_database_upgrade (notmuch, NULL, NULL);
>> -    if (status) {
>> +    if (status)
>>  	notmuch_database_close(notmuch);
>> -	notmuch = NULL;
>> -    }
>>  
>>    DONE:
>>      if (notmuch_path)
>>  	talloc_free (notmuch_path);
>>  
>> -    if (database)
>> -	*database = notmuch;
>> -    else
>> -	talloc_free (notmuch);
>>      return status;
>>  }
>>  
>> @@ -612,14 +616,15 @@ _notmuch_database_ensure_writable (notmuch_database_t *notmuch)
>>      return NOTMUCH_STATUS_SUCCESS;
>>  }
>>  
>> +/*
>> + * XXX: error handling should clean up *all* state created!
>> + */
>
> I think the only thing that will currently leak from this in an error
> case is notmuch->path.
>
>>  notmuch_status_t
>> -notmuch_database_open (const char *path,
>> -		       notmuch_database_mode_t mode,
>> -		       notmuch_database_t **database)
>> +notmuch_database_open (notmuch_database_t *notmuch, const char *path,
>> +		       notmuch_database_mode_t mode)
>>  {
>
> This should also fail if passed a database that is already open.
>
>>      notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
>>      void *local = talloc_new (NULL);
>> -    notmuch_database_t *notmuch = NULL;
>>      char *notmuch_path, *xapian_path;
>>      struct stat st;
>>      int err;
>> @@ -663,7 +668,6 @@ notmuch_database_open (const char *path,
>>  	initialized = 1;
>>      }
>>  
>> -    notmuch = talloc_zero (NULL, notmuch_database_t);
>>      notmuch->exception_reported = FALSE;
>>      notmuch->path = talloc_strdup (notmuch, path);
>>  
>> @@ -689,8 +693,7 @@ notmuch_database_open (const char *path,
>>  			 "       read-write mode.\n",
>>  			 notmuch_path, version, NOTMUCH_DATABASE_VERSION);
>>  		notmuch->mode = NOTMUCH_DATABASE_MODE_READ_ONLY;
>> -		notmuch_database_destroy (notmuch);
>> -		notmuch = NULL;
>> +		notmuch_database_close (notmuch);
>>  		status = NOTMUCH_STATUS_FILE_ERROR;
>>  		goto DONE;
>>  	    }
>> @@ -752,21 +755,19 @@ notmuch_database_open (const char *path,
>>      } catch (const Xapian::Error &error) {
>>  	fprintf (stderr, "A Xapian exception occurred opening database: %s\n",
>>  		 error.get_msg().c_str());
>> -	notmuch_database_destroy (notmuch);
>> -	notmuch = NULL;
>> +	notmuch_database_close (notmuch);
>>  	status = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
>>      }
>>  
>>    DONE:
>>      talloc_free (local);
>>  
>
> It might be simpler to call notmuch_database_close here if status !=
> NOTMUCH_STATUS_SUCCESS, rather than calling it at several places above
> (and not on all error paths).
>
>> -    if (database)
>> -	*database = notmuch;
>> -    else
>> -	talloc_free (notmuch);
>>      return status;
>>  }
>>  
>> +/*
>> + * XXX: close should clean up *all* state created by open/create!
>> + */
>
> I believe the only thing it doesn't clean up is path.  (Note that
> cleaning up path here doesn't currently negate the need to clean up
> path above, though if you float the close call to the DONE path, it
> would suffice.)
>
>>  notmuch_status_t
>>  notmuch_database_close (notmuch_database_t *notmuch)
>>  {
>> @@ -869,7 +870,8 @@ public:
>>   * compaction process to protect data integrity.
>>   */
>>  notmuch_status_t
>> -notmuch_database_compact (const char *path,
>> +notmuch_database_compact (notmuch_database_t *notmuch,
>> +			  const char *path,
>>  			  const char *backup_path,
>>  			  notmuch_compact_status_cb_t status_cb,
>>  			  void *closure)
>> @@ -877,7 +879,6 @@ notmuch_database_compact (const char *path,
>>      void *local;
>>      char *notmuch_path, *xapian_path, *compact_xapian_path;
>>      notmuch_status_t ret = NOTMUCH_STATUS_SUCCESS;
>> -    notmuch_database_t *notmuch = NULL;
>>      struct stat statbuf;
>>      notmuch_bool_t keep_backup;
>>  
>> @@ -885,7 +886,8 @@ notmuch_database_compact (const char *path,
>>      if (! local)
>>  	return NOTMUCH_STATUS_OUT_OF_MEMORY;
>>  
>> -    ret = notmuch_database_open (path, NOTMUCH_DATABASE_MODE_READ_WRITE, &notmuch);
>> +    ret = notmuch_database_open (notmuch, path,
>> +				 NOTMUCH_DATABASE_MODE_READ_WRITE);
>>      if (ret) {
>>  	goto DONE;
>>      }
>> @@ -971,8 +973,9 @@ notmuch_database_compact (const char *path,
>>      }
>>  
>>    DONE:
>> +    /* XXX: error handling */
>>      if (notmuch)
>> -	ret = notmuch_database_destroy (notmuch);
>> +	ret = notmuch_database_close (notmuch);
>>  
>>      talloc_free (local);
>>  
>> @@ -980,7 +983,8 @@ notmuch_database_compact (const char *path,
>>  }
>>  #else
>>  notmuch_status_t
>> -notmuch_database_compact (unused (const char *path),
>> +notmuch_database_compact (unused (notmuch_database_t *notmuch),
>> +			  unused (const char *path),
>>  			  unused (const char *backup_path),
>>  			  unused (notmuch_compact_status_cb_t status_cb),
>>  			  unused (void *closure))
>> diff --git a/lib/notmuch.h b/lib/notmuch.h
>> index dbdce86..cd58d15 100644
>> --- a/lib/notmuch.h
>> +++ b/lib/notmuch.h
>> @@ -149,6 +149,28 @@ typedef struct _notmuch_tags notmuch_tags_t;
>>  typedef struct _notmuch_directory notmuch_directory_t;
>>  typedef struct _notmuch_filenames notmuch_filenames_t;
>>  
>> +/* Initialize a new, empty database handle.
>> + *
>> + * The database handle is required for creating, opening, and
>> + * compacting a database. For further database operations, the
>> + * database needs to be created or opened.
>> + *
>> + * After a successful call to notmuch_database_new, the returned
>> + * database handle will remain in memory, so the caller should call
>> + * notmuch_database_destroy when finished with the database handle.
>> + *
>> + * In case of any failure, this function returns an error status and
>> + * sets *notmuch to NULL.
>> + *
>> + * Return value:
>> + *
>> + * NOTMUCH_STATUS_SUCCESS: Successfully created the database object.
>> + *
>> + * NOTMUCH_STATUS_OUT_OF_MEMORY: Out of memory.
>> + */
>> +notmuch_status_t
>> +notmuch_database_new (notmuch_database_t **notmuch);
>> +
>>  /* Create a new, empty notmuch database located at 'path'.
>>   *
>>   * The path should be a top-level directory to a collection of
>> @@ -156,9 +178,9 @@ typedef struct _notmuch_filenames notmuch_filenames_t;
>>   * create a new ".notmuch" directory within 'path' where notmuch will
>>   * store its data.
>>   *
>> - * After a successful call to notmuch_database_create, the returned
>> - * database will be open so the caller should call
>> - * notmuch_database_destroy when finished with it.
>> + * After a successful call to notmuch_database_create, the database
>> + * will be open so the caller should call notmuch_database_close (or
>> + * notmuch_database_destroy) when finished with the database.
>>   *
>>   * The database will not yet have any data in it
>>   * (notmuch_database_create itself is a very cheap function). Messages
>> @@ -166,7 +188,8 @@ typedef struct _notmuch_filenames notmuch_filenames_t;
>>   * notmuch_database_add_message.
>>   *
>>   * In case of any failure, this function returns an error status and
>> - * sets *database to NULL (after printing an error message on stderr).
>> + * the database will be closed (after printing an error message on
>> + * stderr).
>>   *
>>   * Return value:
>>   *
>> @@ -183,7 +206,7 @@ typedef struct _notmuch_filenames notmuch_filenames_t;
>>   * NOTMUCH_STATUS_XAPIAN_EXCEPTION: A Xapian exception occurred.
>>   */
>>  notmuch_status_t
>> -notmuch_database_create (const char *path, notmuch_database_t **database);
>> +notmuch_database_create (notmuch_database_t *notmuch, const char *path);
>>  
>>  typedef enum {
>>      NOTMUCH_DATABASE_MODE_READ_ONLY = 0,
>> @@ -201,11 +224,13 @@ typedef enum {
>>   * An existing notmuch database can be identified by the presence of a
>>   * directory named ".notmuch" below 'path'.
>>   *
>> - * The caller should call notmuch_database_destroy when finished with
>> - * this database.
>> + * After a successful call to notmuch_database_open, the database will
>> + * be open so the caller should call notmuch_database_close (or
>> + * notmuch_database_destroy) when finished with the database.
>>   *
>>   * In case of any failure, this function returns an error status and
>> - * sets *database to NULL (after printing an error message on stderr).
>> + * the database will be closed (after printing an error message on
>> + * stderr).
>>   *
>>   * Return value:
>>   *
>> @@ -222,9 +247,8 @@ typedef enum {
>>   * NOTMUCH_STATUS_XAPIAN_EXCEPTION: A Xapian exception occurred.
>>   */
>>  notmuch_status_t
>> -notmuch_database_open (const char *path,
>> -		       notmuch_database_mode_t mode,
>> -		       notmuch_database_t **database);
>> +notmuch_database_open (notmuch_database_t *notmuch, const char *path,
>> +		       notmuch_database_mode_t mode);
>>  
>>  /* Close the given notmuch database.
>>   *
>> @@ -264,7 +288,8 @@ typedef void (*notmuch_compact_status_cb_t)(const char *message, void *closure);
>>   * 'closure' is passed verbatim to any callback invoked.
>>   */
>>  notmuch_status_t
>> -notmuch_database_compact (const char* path,
>> +notmuch_database_compact (notmuch_database_t *notmuch,
>> +			  const char* path,
>>  			  const char* backup_path,
>>  			  notmuch_compact_status_cb_t status_cb,
>>  			  void *closure);
>> @@ -272,6 +297,9 @@ notmuch_database_compact (const char* path,
>>  /* Destroy the notmuch database, closing it if necessary and freeing
>>   * all associated resources.
>>   *
>> + * A database handle initialized with notmuch_database_new should be
>> + * destroyed by calling notmuch_database_destroy.
>> + *
>>   * Return value as in notmuch_database_close if the database was open;
>>   * notmuch_database_destroy itself has no failure modes.
>>   */
>> diff --git a/notmuch-compact.c b/notmuch-compact.c
>> index 8b820c0..626685e 100644
>> --- a/notmuch-compact.c
>> +++ b/notmuch-compact.c
>> @@ -29,6 +29,7 @@ status_update_cb (const char *msg, unused (void *closure))
>>  int
>>  notmuch_compact_command (notmuch_config_t *config, int argc, char *argv[])
>>  {
>> +    notmuch_database_t *notmuch = NULL;
>>      const char *path = notmuch_config_get_database_path (config);
>>      const char *backup_path = NULL;
>>      notmuch_status_t ret;
>> @@ -46,7 +47,13 @@ notmuch_compact_command (notmuch_config_t *config, int argc, char *argv[])
>>  
>>      if (! quiet)
>>  	printf ("Compacting database...\n");
>> -    ret = notmuch_database_compact (path, backup_path,
>> +
>> +    if (notmuch_database_new (&notmuch)) {
>> +	fprintf (stderr, "Out of memory\n");
>> +	return 1;
>> +    }
>> +
>> +    ret = notmuch_database_compact (notmuch, path, backup_path,
>>  				    quiet ? NULL : status_update_cb, NULL);
>>      if (ret) {
>>  	fprintf (stderr, "Compaction failed: %s\n", notmuch_status_to_string (ret));
>> @@ -60,5 +67,7 @@ notmuch_compact_command (notmuch_config_t *config, int argc, char *argv[])
>>  	printf ("Done.\n");
>>      }
>>  
>> +    notmuch_database_destroy (notmuch);
>> +
>>      return 0;
>>  }
>> diff --git a/notmuch-count.c b/notmuch-count.c
>> index 01e4e30..15c95c7 100644
>> --- a/notmuch-count.c
>> +++ b/notmuch-count.c
>> @@ -170,8 +170,14 @@ notmuch_count_command (notmuch_config_t *config, int argc, char *argv[])
>>  	return 1;
>>      }
>>  
>> -    if (notmuch_database_open (notmuch_config_get_database_path (config),
>> -			       NOTMUCH_DATABASE_MODE_READ_ONLY, &notmuch))
>> +    if (notmuch_database_new (&notmuch)) {
>> +	fprintf (stderr, "Out of memory\n");
>> +	return 1;
>> +    }
>> +
>> +    if (notmuch_database_open (notmuch,
>> +			       notmuch_config_get_database_path (config),
>> +			       NOTMUCH_DATABASE_MODE_READ_ONLY))
>
> Does this need to destroy the database?  (Likewise for all the others.)
>
>>  	return 1;
>>  
>>      query_str = query_string_from_args (config, argc-opt_index, argv+opt_index);
>> diff --git a/notmuch-dump.c b/notmuch-dump.c
>> index 2024e30..73579bc 100644
>> --- a/notmuch-dump.c
>> +++ b/notmuch-dump.c
>> @@ -33,8 +33,14 @@ notmuch_dump_command (notmuch_config_t *config, int argc, char *argv[])
>>      notmuch_tags_t *tags;
>>      const char *query_str = "";
>>  
>> -    if (notmuch_database_open (notmuch_config_get_database_path (config),
>> -			       NOTMUCH_DATABASE_MODE_READ_ONLY, &notmuch))
>> +    if (notmuch_database_new (&notmuch)) {
>> +	fprintf (stderr, "Out of memory\n");
>> +	return 1;
>> +    }
>> +
>> +    if (notmuch_database_open (notmuch,
>> +			       notmuch_config_get_database_path (config),
>> +			       NOTMUCH_DATABASE_MODE_READ_ONLY))
>>  	return 1;
>>  
>>      char *output_file_name = NULL;
>> diff --git a/notmuch-insert.c b/notmuch-insert.c
>> index 2207b1e..4a8aad3 100644
>> --- a/notmuch-insert.c
>> +++ b/notmuch-insert.c
>> @@ -467,8 +467,14 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
>>      action.sa_flags = 0;
>>      sigaction (SIGINT, &action, NULL);
>>  
>> -    if (notmuch_database_open (notmuch_config_get_database_path (config),
>> -			       NOTMUCH_DATABASE_MODE_READ_WRITE, &notmuch))
>> +    if (notmuch_database_new (&notmuch)) {
>> +	fprintf (stderr, "Out of memory\n");
>> +	return 1;
>> +    }
>> +
>> +    if (notmuch_database_open (notmuch,
>> +			       notmuch_config_get_database_path (config),
>> +			       NOTMUCH_DATABASE_MODE_READ_WRITE))
>>  	return 1;
>>  
>>      ret = insert_message (config, notmuch, STDIN_FILENO, maildir, tag_ops);
>> diff --git a/notmuch-new.c b/notmuch-new.c
>> index ba05cb4..f72a4de 100644
>> --- a/notmuch-new.c
>> +++ b/notmuch-new.c
>> @@ -914,6 +914,11 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[])
>>  	    return ret;
>>      }
>>  
>> +    if (notmuch_database_new (&notmuch)) {
>> +	fprintf (stderr, "Out of memory\n");
>> +	return 1;
>> +    }
>> +
>>      dot_notmuch_path = talloc_asprintf (config, "%s/%s", db_path, ".notmuch");
>>  
>>      if (stat (dot_notmuch_path, &st)) {
>> @@ -925,12 +930,12 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[])
>>  	    return 1;
>>  
>>  	printf ("Found %d total files (that's not much mail).\n", count);
>> -	if (notmuch_database_create (db_path, &notmuch))
>> +	if (notmuch_database_create (notmuch, db_path))
>>  	    return 1;
>>  	add_files_state.total_files = count;
>>      } else {
>> -	if (notmuch_database_open (db_path, NOTMUCH_DATABASE_MODE_READ_WRITE,
>> -				   &notmuch))
>> +	if (notmuch_database_open (notmuch, db_path,
>> +				   NOTMUCH_DATABASE_MODE_READ_WRITE))
>>  	    return 1;
>>  
>>  	if (notmuch_database_needs_upgrade (notmuch)) {
>> @@ -945,9 +950,6 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[])
>>  	add_files_state.total_files = 0;
>>      }
>>  
>> -    if (notmuch == NULL)
>> -	return 1;
>> -
>>      /* Setup our handler for SIGINT. We do this after having
>>       * potentially done a database upgrade we this interrupt handler
>>       * won't support. */
>> diff --git a/notmuch-reply.c b/notmuch-reply.c
>> index 9d6f843..7b80841 100644
>> --- a/notmuch-reply.c
>> +++ b/notmuch-reply.c
>> @@ -769,8 +769,14 @@ notmuch_reply_command (notmuch_config_t *config, int argc, char *argv[])
>>  	return 1;
>>      }
>>  
>> -    if (notmuch_database_open (notmuch_config_get_database_path (config),
>> -			       NOTMUCH_DATABASE_MODE_READ_ONLY, &notmuch))
>> +    if (notmuch_database_new (&notmuch)) {
>> +	fprintf (stderr, "Out of memory\n");
>> +	return 1;
>> +    }
>> +
>> +    if (notmuch_database_open (notmuch,
>> +			       notmuch_config_get_database_path (config),
>> +			       NOTMUCH_DATABASE_MODE_READ_ONLY))
>>  	return 1;
>>  
>>      query = notmuch_query_create (notmuch, query_string);
>> diff --git a/notmuch-restore.c b/notmuch-restore.c
>> index 1419621..fc37838 100644
>> --- a/notmuch-restore.c
>> +++ b/notmuch-restore.c
>> @@ -138,8 +138,14 @@ notmuch_restore_command (notmuch_config_t *config, int argc, char *argv[])
>>      int opt_index;
>>      int input_format = DUMP_FORMAT_AUTO;
>>  
>> -    if (notmuch_database_open (notmuch_config_get_database_path (config),
>> -			       NOTMUCH_DATABASE_MODE_READ_WRITE, &notmuch))
>> +    if (notmuch_database_new (&notmuch)) {
>> +	fprintf (stderr, "Out of memory\n");
>> +	return 1;
>> +    }
>> +
>> +    if (notmuch_database_open (notmuch,
>> +			       notmuch_config_get_database_path (config),
>> +			       NOTMUCH_DATABASE_MODE_READ_WRITE))
>>  	return 1;
>>  
>>      if (notmuch_config_get_maildir_synchronize_flags (config))
>> diff --git a/notmuch-search.c b/notmuch-search.c
>> index 7c973b3..50aace9 100644
>> --- a/notmuch-search.c
>> +++ b/notmuch-search.c
>> @@ -430,8 +430,14 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
>>  
>>      notmuch_exit_if_unsupported_format ();
>>  
>> -    if (notmuch_database_open (notmuch_config_get_database_path (config),
>> -			       NOTMUCH_DATABASE_MODE_READ_ONLY, &notmuch))
>> +    if (notmuch_database_new (&notmuch)) {
>> +	fprintf (stderr, "Out of memory\n");
>> +	return 1;
>> +    }
>> +
>> +    if (notmuch_database_open (notmuch,
>> +			       notmuch_config_get_database_path (config),
>> +			       NOTMUCH_DATABASE_MODE_READ_ONLY))
>>  	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 c07f887..bc44b2c 100644
>> --- a/notmuch-show.c
>> +++ b/notmuch-show.c
>> @@ -1201,8 +1201,14 @@ notmuch_show_command (notmuch_config_t *config, int argc, char *argv[])
>>  	return 1;
>>      }
>>  
>> -    if (notmuch_database_open (notmuch_config_get_database_path (config),
>> -			       NOTMUCH_DATABASE_MODE_READ_ONLY, &notmuch))
>> +    if (notmuch_database_new (&notmuch)) {
>> +	fprintf (stderr, "Out of memory\n");
>> +	return 1;
>> +    }
>> +
>> +    if (notmuch_database_open (notmuch,
>> +			       notmuch_config_get_database_path (config),
>> +			       NOTMUCH_DATABASE_MODE_READ_ONLY))
>>  	return 1;
>>  
>>      query = notmuch_query_create (notmuch, query_string);
>> diff --git a/notmuch-tag.c b/notmuch-tag.c
>> index 3b09df9..6e29799 100644
>> --- a/notmuch-tag.c
>> +++ b/notmuch-tag.c
>> @@ -254,8 +254,14 @@ notmuch_tag_command (notmuch_config_t *config, int argc, char *argv[])
>>  	}
>>      }
>>  
>> -    if (notmuch_database_open (notmuch_config_get_database_path (config),
>> -			       NOTMUCH_DATABASE_MODE_READ_WRITE, &notmuch))
>> +    if (notmuch_database_new (&notmuch)) {
>> +	fprintf (stderr, "Out of memory\n");
>> +	return 1;
>> +    }
>> +
>> +    if (notmuch_database_open (notmuch,
>> +			       notmuch_config_get_database_path (config),
>> +			       NOTMUCH_DATABASE_MODE_READ_WRITE))
>>  	return 1;
>>  
>>      if (notmuch_config_get_maildir_synchronize_flags (config))
>> diff --git a/test/random-corpus.c b/test/random-corpus.c
>> index 790193d..2b205e5 100644
>> --- a/test/random-corpus.c
>> +++ b/test/random-corpus.c
>> @@ -164,8 +164,14 @@ main (int argc, char **argv)
>>      if (config == NULL)
>>  	return 1;
>>  
>> -    if (notmuch_database_open (notmuch_config_get_database_path (config),
>> -			       NOTMUCH_DATABASE_MODE_READ_WRITE, &notmuch))
>> +    if (notmuch_database_new (&notmuch)) {
>> +	fprintf (stderr, "Out of memory\n");
>> +	return 1;
>> +    }
>> +
>> +    if (notmuch_database_open (notmuch,
>> +			       notmuch_config_get_database_path (config),
>> +			       NOTMUCH_DATABASE_MODE_READ_WRITE))
>>  	return 1;
>>  
>>      srandom (seed);
>> diff --git a/test/symbol-test.cc b/test/symbol-test.cc
>> index 3e96c03..47c5351 100644
>> --- a/test/symbol-test.cc
>> +++ b/test/symbol-test.cc
>> @@ -5,7 +5,8 @@
>>  
>>  int main() {
>>    notmuch_database_t *notmuch;
>> -  notmuch_database_open("fakedb", NOTMUCH_DATABASE_MODE_READ_ONLY, &notmuch);
>> +  notmuch_database_new (&notmuch);
>> +  notmuch_database_open (notmuch, "fakedb", NOTMUCH_DATABASE_MODE_READ_ONLY);
>>  
>>    try {
>>      (void) new Xapian::WritableDatabase("./nonexistant", Xapian::DB_OPEN);