Re: [PATCH 2/2] lib: introduce notmuch_database_new for initializing a database handle
authorAustin Clements <amdragon@MIT.EDU>
Wed, 4 Dec 2013 23:11:13 +0000 (18:11 +1900)
committerW. Trevor King <wking@tremily.us>
Fri, 7 Nov 2014 17:58:38 +0000 (09:58 -0800)
a4/6560f778037588e9b0da21e18106f3cfc7074d [new file with mode: 0644]

diff --git a/a4/6560f778037588e9b0da21e18106f3cfc7074d b/a4/6560f778037588e9b0da21e18106f3cfc7074d
new file mode 100644 (file)
index 0000000..84013f8
--- /dev/null
@@ -0,0 +1,744 @@
+Return-Path: <amdragon@mit.edu>\r
+X-Original-To: notmuch@notmuchmail.org\r
+Delivered-To: notmuch@notmuchmail.org\r
+Received: from localhost (localhost [127.0.0.1])\r
+       by olra.theworths.org (Postfix) with ESMTP id 491BB431FD2\r
+       for <notmuch@notmuchmail.org>; Wed,  4 Dec 2013 15:11:28 -0800 (PST)\r
+X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
+X-Spam-Flag: NO\r
+X-Spam-Score: -0.7\r
+X-Spam-Level: \r
+X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5\r
+       tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled\r
+Received: from olra.theworths.org ([127.0.0.1])\r
+       by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
+       with ESMTP id OryFt9l6vpw4 for <notmuch@notmuchmail.org>;\r
+       Wed,  4 Dec 2013 15:11:23 -0800 (PST)\r
+Received: from dmz-mailsec-scanner-8.mit.edu (dmz-mailsec-scanner-8.mit.edu\r
+       [18.7.68.37])\r
+       (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits))\r
+       (No client certificate requested)\r
+       by olra.theworths.org (Postfix) with ESMTPS id 7A010431FD0\r
+       for <notmuch@notmuchmail.org>; Wed,  4 Dec 2013 15:11:22 -0800 (PST)\r
+X-AuditID: 12074425-b7fd96d000000c39-7e-529fb698e984\r
+Received: from mailhub-auth-4.mit.edu ( [18.7.62.39])\r
+       (using TLS with cipher AES256-SHA (256/256 bits))\r
+       (Client did not present a certificate)\r
+       by dmz-mailsec-scanner-8.mit.edu (Symantec Messaging Gateway) with SMTP\r
+       id 8F.AE.03129.896BF925; Wed,  4 Dec 2013 18:11:20 -0500 (EST)\r
+Received: from outgoing.mit.edu (outgoing-auth-1.mit.edu [18.9.28.11])\r
+       by mailhub-auth-4.mit.edu (8.13.8/8.9.2) with ESMTP id rB4NBJJ7032395; \r
+       Wed, 4 Dec 2013 18:11:20 -0500\r
+Received: from awakening.csail.mit.edu (awakening.csail.mit.edu [18.26.4.91])\r
+       (authenticated bits=0)\r
+       (User authenticated as amdragon@ATHENA.MIT.EDU)\r
+       by outgoing.mit.edu (8.13.8/8.12.4) with ESMTP id rB4NBGGu022234\r
+       (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT);\r
+       Wed, 4 Dec 2013 18:11:18 -0500\r
+Received: from amthrax by awakening.csail.mit.edu with local (Exim 4.80)\r
+       (envelope-from <amdragon@mit.edu>)\r
+       id 1VoLb7-0002im-TJ; Wed, 04 Dec 2013 18:11:15 -0500\r
+Date: Wed, 4 Dec 2013 18:11:13 -0500\r
+From: Austin Clements <amdragon@MIT.EDU>\r
+To: Jani Nikula <jani@nikula.org>\r
+Subject: Re: [PATCH 2/2] lib: introduce notmuch_database_new for initializing\r
+       a database handle\r
+Message-ID: <20131204231113.GD8854@mit.edu>\r
+References: <cover.1385903109.git.jani@nikula.org>\r
+       <fc7ecd990e55fcfba17de4d71e8823c98760f9ce.1385903109.git.jani@nikula.org>\r
+MIME-Version: 1.0\r
+Content-Type: text/plain; charset=us-ascii\r
+Content-Disposition: inline\r
+In-Reply-To:\r
+ <fc7ecd990e55fcfba17de4d71e8823c98760f9ce.1385903109.git.jani@nikula.org>\r
+User-Agent: Mutt/1.5.21 (2010-09-15)\r
+X-Brightmail-Tracker:\r
+ H4sIAAAAAAAAA+NgFmplleLIzCtJLcpLzFFi42IRYrdT152xbX6QwawPKhZN050trt+cyezA\r
+       5HHr/mt2j2erbjEHMEVx2aSk5mSWpRbp2yVwZex5uImloPsIY8WEjuuMDYyPZzJ2MXJySAiY\r
+       SLy9fw3KFpO4cG89WxcjF4eQwGwmic7PW5ghnA2MEr+71rBCOKeYJC6tvcAI4SxhlNjw/T8z\r
+       SD+LgIpE/7ElbCA2m4CGxLb9y8HmiggoSmw+uR/MZhaQlvj2u5kJxBYWSJS4unkvWD2vgLbE\r
+       /fZ2VhBbSKBOYt66fqi4oMTJmU9YIHq1JG78ewnUywE2Z/k/DpAwp0CYxI2OxewgtijQCVNO\r
+       bmObwCg0C0n3LCTdsxC6FzAyr2KUTcmt0s1NzMwpTk3WLU5OzMtLLdK10MvNLNFLTSndxAgO\r
+       bRfVHYwTDikdYhTgYFTi4XVImR8kxJpYVlyZe4hRkoNJSZTXZiNQiC8pP6UyI7E4I76oNCe1\r
+       +BCjBAezkgjvvxqgHG9KYmVValE+TEqag0VJnPcWh32QkEB6YklqdmpqQWoRTFaGg0NJgvfH\r
+       VqBGwaLU9NSKtMycEoQ0EwcnyHAeoOGXQWp4iwsSc4sz0yHypxgVpcR580ASAiCJjNI8uF5Y\r
+       6nnFKA70ijDvbZAqHmDagut+BTSYCWhw84N5IINLEhFSUg2MzTu/Z7Zmthk8mjLZhFF+d7Pr\r
+       1EPlktd5+ucGNt2qlssvWzbP8aWnQkbirMDm7k4PnTt35WO2aS4WLhDVvfZp3wHnD8EvDc8m\r
+       54XaxLpKH93R5Pgn26Xsi1zNknMfroW0+lgt41plNiW+hC2O63Tlp/pnV4VW822vXsP7btuB\r
+       z1d6Txs8FStRYinOSDTUYi4qTgQAf5Qg3RgDAAA=\r
+Cc: notmuch@notmuchmail.org\r
+X-BeenThere: notmuch@notmuchmail.org\r
+X-Mailman-Version: 2.1.13\r
+Precedence: list\r
+List-Id: "Use and development of the notmuch mail system."\r
+       <notmuch.notmuchmail.org>\r
+List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
+       <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
+List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
+List-Post: <mailto:notmuch@notmuchmail.org>\r
+List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
+List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
+       <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
+X-List-Received-Date: Wed, 04 Dec 2013 23:11:28 -0000\r
+\r
+Quoth Jani Nikula on Dec 01 at  3:14 pm:\r
+> There is a need for setting options before opening a database, such as\r
+> setting a logging function to use instead of writing to stdout or\r
+> stderr. It would be possible to do this by adding new parameters to\r
+> notmuch_database_create and notmuch_database_open, but maintaining a\r
+> backwards compatible API and ABI when new options are added becomes\r
+> burdensome.\r
+> \r
+> Instead, split the opaque database object creation from\r
+> notmuch_database_create and notmuch_database_open into a new\r
+> notmuch_database_new call, to allow operations on the handle before\r
+> create and open. This creates API and ABI breakage now, but allows\r
+> easier future extensions.\r
+> \r
+> The notmuch_database_new call becomes a natural pair to the already\r
+> existing notmuch_database_destroy, and it should be possible to call\r
+> open/close multiple times using an initialized handle.\r
+\r
+A high-level comment about the API: Currently, an allocated\r
+notmuch_database_t has two "memory states", if you will: open and\r
+closed.  (I wish it didn't have any memory states, and was on the\r
+fence about this API for a while until I realized that the ship had\r
+already sailed.)  It's pretty clear how all of the notmuch APIs will\r
+behave in both states (modulo some bounded non-determinism in the\r
+closed state).  I think this patch introduces a new "pre-open" state,\r
+and I don't know how most of the notmuch APIs behave in that state.\r
+My guess is poorly.  If it's feasible, I'd much rather a fresh baked\r
+notmuch_database_t act like it's in the closed state, including that\r
+notmuch_database_{create,open} are well-defined as transitions from\r
+closed state to open state (even if the closed state was reached by\r
+calling notmuch_database_close).  Or, if we do have a "pre-open"\r
+state, it should at least be well-specified what that means\r
+(preferably the specification is *not* "most APIs segfault").\r
+\r
+Orthogonally -- and this may be a complete pipe dream of mine -- if we\r
+just had a way to return more detailed error information than a simple\r
+error code from notmuch_database_{create,open}, I think we wouldn't\r
+need any of this.  Everything that these functions currently log\r
+(modulo one warning) is error details, so if we could return the error\r
+details *with the error* or somehow make them accessible, we wouldn't\r
+need a logger at this point (or at several other points in the\r
+library).\r
+\r
+> ---\r
+>  lib/database.cc      | 64 ++++++++++++++++++++++++++++------------------------\r
+>  lib/notmuch.h        | 52 ++++++++++++++++++++++++++++++++----------\r
+>  notmuch-compact.c    | 11 ++++++++-\r
+>  notmuch-count.c      | 10 ++++++--\r
+>  notmuch-dump.c       | 10 ++++++--\r
+>  notmuch-insert.c     | 10 ++++++--\r
+>  notmuch-new.c        | 14 +++++++-----\r
+>  notmuch-reply.c      | 10 ++++++--\r
+>  notmuch-restore.c    | 10 ++++++--\r
+>  notmuch-search.c     | 10 ++++++--\r
+>  notmuch-show.c       | 10 ++++++--\r
+>  notmuch-tag.c        | 10 ++++++--\r
+>  test/random-corpus.c | 10 ++++++--\r
+>  test/symbol-test.cc  |  3 ++-\r
+>  14 files changed, 166 insertions(+), 68 deletions(-)\r
+> \r
+> diff --git a/lib/database.cc b/lib/database.cc\r
+> index 98e2c31..386b93a 100644\r
+> --- a/lib/database.cc\r
+> +++ b/lib/database.cc\r
+> @@ -539,10 +539,21 @@ parse_references (void *ctx,\r
+>  }\r
+>  \r
+>  notmuch_status_t\r
+> -notmuch_database_create (const char *path, notmuch_database_t **database)\r
+> +notmuch_database_new (notmuch_database_t **notmuch)\r
+\r
+The naming of this is unfortunate...  Other APIs use x_create to\r
+allocate objects (e.g., notmuch_query_create, several internal APIs).\r
+I would lean towards calling this function notmuch_database_create,\r
+but that leaves the question of what to call the other.  While we're\r
+breaking APIs, would it be completely crazy to merge open and create\r
+into one API with an extra mode to indicate creation (it can be its\r
+own mode because creation implies read/write)?  (Or, in UNIX\r
+tradition, we could call this function notmuch_database_create and the\r
+other notmuch_database_creat.)  notmuch_database_create is already\r
+just a shell around notmuch_database_open (we could keep it as a\r
+separate function, but just make it internal).\r
+\r
+> +{\r
+> +    /* Note: try to avoid error conditions! No error printing! */\r
+> +\r
+> +    *notmuch = talloc_zero (NULL, notmuch_database_t);\r
+> +    if (! *notmuch)\r
+> +    return NOTMUCH_STATUS_OUT_OF_MEMORY;\r
+> +\r
+> +    return NOTMUCH_STATUS_SUCCESS;\r
+> +}\r
+> +\r
+> +notmuch_status_t\r
+> +notmuch_database_create (notmuch_database_t *notmuch, const char *path)\r
+>  {\r
+\r
+This should fail if passed a database that is already open.\r
+\r
+>      notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;\r
+> -    notmuch_database_t *notmuch = NULL;\r
+>      char *notmuch_path = NULL;\r
+>      struct stat st;\r
+>      int err;\r
+> @@ -579,25 +590,18 @@ notmuch_database_create (const char *path, notmuch_database_t **database)\r
+>      goto DONE;\r
+>      }\r
+>  \r
+> -    status = notmuch_database_open (path,\r
+> -                                NOTMUCH_DATABASE_MODE_READ_WRITE,\r
+> -                                &notmuch);\r
+> +    status = notmuch_database_open (notmuch, path,\r
+> +                                NOTMUCH_DATABASE_MODE_READ_WRITE);\r
+>      if (status)\r
+>      goto DONE;\r
+>      status = notmuch_database_upgrade (notmuch, NULL, NULL);\r
+> -    if (status) {\r
+> +    if (status)\r
+>      notmuch_database_close(notmuch);\r
+> -    notmuch = NULL;\r
+> -    }\r
+>  \r
+>    DONE:\r
+>      if (notmuch_path)\r
+>      talloc_free (notmuch_path);\r
+>  \r
+> -    if (database)\r
+> -    *database = notmuch;\r
+> -    else\r
+> -    talloc_free (notmuch);\r
+>      return status;\r
+>  }\r
+>  \r
+> @@ -612,14 +616,15 @@ _notmuch_database_ensure_writable (notmuch_database_t *notmuch)\r
+>      return NOTMUCH_STATUS_SUCCESS;\r
+>  }\r
+>  \r
+> +/*\r
+> + * XXX: error handling should clean up *all* state created!\r
+> + */\r
+\r
+I think the only thing that will currently leak from this in an error\r
+case is notmuch->path.\r
+\r
+>  notmuch_status_t\r
+> -notmuch_database_open (const char *path,\r
+> -                   notmuch_database_mode_t mode,\r
+> -                   notmuch_database_t **database)\r
+> +notmuch_database_open (notmuch_database_t *notmuch, const char *path,\r
+> +                   notmuch_database_mode_t mode)\r
+>  {\r
+\r
+This should also fail if passed a database that is already open.\r
+\r
+>      notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;\r
+>      void *local = talloc_new (NULL);\r
+> -    notmuch_database_t *notmuch = NULL;\r
+>      char *notmuch_path, *xapian_path;\r
+>      struct stat st;\r
+>      int err;\r
+> @@ -663,7 +668,6 @@ notmuch_database_open (const char *path,\r
+>      initialized = 1;\r
+>      }\r
+>  \r
+> -    notmuch = talloc_zero (NULL, notmuch_database_t);\r
+>      notmuch->exception_reported = FALSE;\r
+>      notmuch->path = talloc_strdup (notmuch, path);\r
+>  \r
+> @@ -689,8 +693,7 @@ notmuch_database_open (const char *path,\r
+>                       "       read-write mode.\n",\r
+>                       notmuch_path, version, NOTMUCH_DATABASE_VERSION);\r
+>              notmuch->mode = NOTMUCH_DATABASE_MODE_READ_ONLY;\r
+> -            notmuch_database_destroy (notmuch);\r
+> -            notmuch = NULL;\r
+> +            notmuch_database_close (notmuch);\r
+>              status = NOTMUCH_STATUS_FILE_ERROR;\r
+>              goto DONE;\r
+>          }\r
+> @@ -752,21 +755,19 @@ notmuch_database_open (const char *path,\r
+>      } catch (const Xapian::Error &error) {\r
+>      fprintf (stderr, "A Xapian exception occurred opening database: %s\n",\r
+>               error.get_msg().c_str());\r
+> -    notmuch_database_destroy (notmuch);\r
+> -    notmuch = NULL;\r
+> +    notmuch_database_close (notmuch);\r
+>      status = NOTMUCH_STATUS_XAPIAN_EXCEPTION;\r
+>      }\r
+>  \r
+>    DONE:\r
+>      talloc_free (local);\r
+>  \r
+\r
+It might be simpler to call notmuch_database_close here if status !=\r
+NOTMUCH_STATUS_SUCCESS, rather than calling it at several places above\r
+(and not on all error paths).\r
+\r
+> -    if (database)\r
+> -    *database = notmuch;\r
+> -    else\r
+> -    talloc_free (notmuch);\r
+>      return status;\r
+>  }\r
+>  \r
+> +/*\r
+> + * XXX: close should clean up *all* state created by open/create!\r
+> + */\r
+\r
+I believe the only thing it doesn't clean up is path.  (Note that\r
+cleaning up path here doesn't currently negate the need to clean up\r
+path above, though if you float the close call to the DONE path, it\r
+would suffice.)\r
+\r
+>  notmuch_status_t\r
+>  notmuch_database_close (notmuch_database_t *notmuch)\r
+>  {\r
+> @@ -869,7 +870,8 @@ public:\r
+>   * compaction process to protect data integrity.\r
+>   */\r
+>  notmuch_status_t\r
+> -notmuch_database_compact (const char *path,\r
+> +notmuch_database_compact (notmuch_database_t *notmuch,\r
+> +                      const char *path,\r
+>                        const char *backup_path,\r
+>                        notmuch_compact_status_cb_t status_cb,\r
+>                        void *closure)\r
+> @@ -877,7 +879,6 @@ notmuch_database_compact (const char *path,\r
+>      void *local;\r
+>      char *notmuch_path, *xapian_path, *compact_xapian_path;\r
+>      notmuch_status_t ret = NOTMUCH_STATUS_SUCCESS;\r
+> -    notmuch_database_t *notmuch = NULL;\r
+>      struct stat statbuf;\r
+>      notmuch_bool_t keep_backup;\r
+>  \r
+> @@ -885,7 +886,8 @@ notmuch_database_compact (const char *path,\r
+>      if (! local)\r
+>      return NOTMUCH_STATUS_OUT_OF_MEMORY;\r
+>  \r
+> -    ret = notmuch_database_open (path, NOTMUCH_DATABASE_MODE_READ_WRITE, &notmuch);\r
+> +    ret = notmuch_database_open (notmuch, path,\r
+> +                             NOTMUCH_DATABASE_MODE_READ_WRITE);\r
+>      if (ret) {\r
+>      goto DONE;\r
+>      }\r
+> @@ -971,8 +973,9 @@ notmuch_database_compact (const char *path,\r
+>      }\r
+>  \r
+>    DONE:\r
+> +    /* XXX: error handling */\r
+>      if (notmuch)\r
+> -    ret = notmuch_database_destroy (notmuch);\r
+> +    ret = notmuch_database_close (notmuch);\r
+>  \r
+>      talloc_free (local);\r
+>  \r
+> @@ -980,7 +983,8 @@ notmuch_database_compact (const char *path,\r
+>  }\r
+>  #else\r
+>  notmuch_status_t\r
+> -notmuch_database_compact (unused (const char *path),\r
+> +notmuch_database_compact (unused (notmuch_database_t *notmuch),\r
+> +                      unused (const char *path),\r
+>                        unused (const char *backup_path),\r
+>                        unused (notmuch_compact_status_cb_t status_cb),\r
+>                        unused (void *closure))\r
+> diff --git a/lib/notmuch.h b/lib/notmuch.h\r
+> index dbdce86..cd58d15 100644\r
+> --- a/lib/notmuch.h\r
+> +++ b/lib/notmuch.h\r
+> @@ -149,6 +149,28 @@ typedef struct _notmuch_tags notmuch_tags_t;\r
+>  typedef struct _notmuch_directory notmuch_directory_t;\r
+>  typedef struct _notmuch_filenames notmuch_filenames_t;\r
+>  \r
+> +/* Initialize a new, empty database handle.\r
+> + *\r
+> + * The database handle is required for creating, opening, and\r
+> + * compacting a database. For further database operations, the\r
+> + * database needs to be created or opened.\r
+> + *\r
+> + * After a successful call to notmuch_database_new, the returned\r
+> + * database handle will remain in memory, so the caller should call\r
+> + * notmuch_database_destroy when finished with the database handle.\r
+> + *\r
+> + * In case of any failure, this function returns an error status and\r
+> + * sets *notmuch to NULL.\r
+> + *\r
+> + * Return value:\r
+> + *\r
+> + * NOTMUCH_STATUS_SUCCESS: Successfully created the database object.\r
+> + *\r
+> + * NOTMUCH_STATUS_OUT_OF_MEMORY: Out of memory.\r
+> + */\r
+> +notmuch_status_t\r
+> +notmuch_database_new (notmuch_database_t **notmuch);\r
+> +\r
+>  /* Create a new, empty notmuch database located at 'path'.\r
+>   *\r
+>   * The path should be a top-level directory to a collection of\r
+> @@ -156,9 +178,9 @@ typedef struct _notmuch_filenames notmuch_filenames_t;\r
+>   * create a new ".notmuch" directory within 'path' where notmuch will\r
+>   * store its data.\r
+>   *\r
+> - * After a successful call to notmuch_database_create, the returned\r
+> - * database will be open so the caller should call\r
+> - * notmuch_database_destroy when finished with it.\r
+> + * After a successful call to notmuch_database_create, the database\r
+> + * will be open so the caller should call notmuch_database_close (or\r
+> + * notmuch_database_destroy) when finished with the database.\r
+>   *\r
+>   * The database will not yet have any data in it\r
+>   * (notmuch_database_create itself is a very cheap function). Messages\r
+> @@ -166,7 +188,8 @@ typedef struct _notmuch_filenames notmuch_filenames_t;\r
+>   * notmuch_database_add_message.\r
+>   *\r
+>   * In case of any failure, this function returns an error status and\r
+> - * sets *database to NULL (after printing an error message on stderr).\r
+> + * the database will be closed (after printing an error message on\r
+> + * stderr).\r
+>   *\r
+>   * Return value:\r
+>   *\r
+> @@ -183,7 +206,7 @@ typedef struct _notmuch_filenames notmuch_filenames_t;\r
+>   * NOTMUCH_STATUS_XAPIAN_EXCEPTION: A Xapian exception occurred.\r
+>   */\r
+>  notmuch_status_t\r
+> -notmuch_database_create (const char *path, notmuch_database_t **database);\r
+> +notmuch_database_create (notmuch_database_t *notmuch, const char *path);\r
+>  \r
+>  typedef enum {\r
+>      NOTMUCH_DATABASE_MODE_READ_ONLY = 0,\r
+> @@ -201,11 +224,13 @@ typedef enum {\r
+>   * An existing notmuch database can be identified by the presence of a\r
+>   * directory named ".notmuch" below 'path'.\r
+>   *\r
+> - * The caller should call notmuch_database_destroy when finished with\r
+> - * this database.\r
+> + * After a successful call to notmuch_database_open, the database will\r
+> + * be open so the caller should call notmuch_database_close (or\r
+> + * notmuch_database_destroy) when finished with the database.\r
+>   *\r
+>   * In case of any failure, this function returns an error status and\r
+> - * sets *database to NULL (after printing an error message on stderr).\r
+> + * the database will be closed (after printing an error message on\r
+> + * stderr).\r
+>   *\r
+>   * Return value:\r
+>   *\r
+> @@ -222,9 +247,8 @@ typedef enum {\r
+>   * NOTMUCH_STATUS_XAPIAN_EXCEPTION: A Xapian exception occurred.\r
+>   */\r
+>  notmuch_status_t\r
+> -notmuch_database_open (const char *path,\r
+> -                   notmuch_database_mode_t mode,\r
+> -                   notmuch_database_t **database);\r
+> +notmuch_database_open (notmuch_database_t *notmuch, const char *path,\r
+> +                   notmuch_database_mode_t mode);\r
+>  \r
+>  /* Close the given notmuch database.\r
+>   *\r
+> @@ -264,7 +288,8 @@ typedef void (*notmuch_compact_status_cb_t)(const char *message, void *closure);\r
+>   * 'closure' is passed verbatim to any callback invoked.\r
+>   */\r
+>  notmuch_status_t\r
+> -notmuch_database_compact (const char* path,\r
+> +notmuch_database_compact (notmuch_database_t *notmuch,\r
+> +                      const char* path,\r
+>                        const char* backup_path,\r
+>                        notmuch_compact_status_cb_t status_cb,\r
+>                        void *closure);\r
+> @@ -272,6 +297,9 @@ notmuch_database_compact (const char* path,\r
+>  /* Destroy the notmuch database, closing it if necessary and freeing\r
+>   * all associated resources.\r
+>   *\r
+> + * A database handle initialized with notmuch_database_new should be\r
+> + * destroyed by calling notmuch_database_destroy.\r
+> + *\r
+>   * Return value as in notmuch_database_close if the database was open;\r
+>   * notmuch_database_destroy itself has no failure modes.\r
+>   */\r
+> diff --git a/notmuch-compact.c b/notmuch-compact.c\r
+> index 8b820c0..626685e 100644\r
+> --- a/notmuch-compact.c\r
+> +++ b/notmuch-compact.c\r
+> @@ -29,6 +29,7 @@ status_update_cb (const char *msg, unused (void *closure))\r
+>  int\r
+>  notmuch_compact_command (notmuch_config_t *config, int argc, char *argv[])\r
+>  {\r
+> +    notmuch_database_t *notmuch = NULL;\r
+>      const char *path = notmuch_config_get_database_path (config);\r
+>      const char *backup_path = NULL;\r
+>      notmuch_status_t ret;\r
+> @@ -46,7 +47,13 @@ notmuch_compact_command (notmuch_config_t *config, int argc, char *argv[])\r
+>  \r
+>      if (! quiet)\r
+>      printf ("Compacting database...\n");\r
+> -    ret = notmuch_database_compact (path, backup_path,\r
+> +\r
+> +    if (notmuch_database_new (&notmuch)) {\r
+> +    fprintf (stderr, "Out of memory\n");\r
+> +    return 1;\r
+> +    }\r
+> +\r
+> +    ret = notmuch_database_compact (notmuch, path, backup_path,\r
+>                                  quiet ? NULL : status_update_cb, NULL);\r
+>      if (ret) {\r
+>      fprintf (stderr, "Compaction failed: %s\n", notmuch_status_to_string (ret));\r
+> @@ -60,5 +67,7 @@ notmuch_compact_command (notmuch_config_t *config, int argc, char *argv[])\r
+>      printf ("Done.\n");\r
+>      }\r
+>  \r
+> +    notmuch_database_destroy (notmuch);\r
+> +\r
+>      return 0;\r
+>  }\r
+> diff --git a/notmuch-count.c b/notmuch-count.c\r
+> index 01e4e30..15c95c7 100644\r
+> --- a/notmuch-count.c\r
+> +++ b/notmuch-count.c\r
+> @@ -170,8 +170,14 @@ notmuch_count_command (notmuch_config_t *config, int argc, char *argv[])\r
+>      return 1;\r
+>      }\r
+>  \r
+> -    if (notmuch_database_open (notmuch_config_get_database_path (config),\r
+> -                           NOTMUCH_DATABASE_MODE_READ_ONLY, &notmuch))\r
+> +    if (notmuch_database_new (&notmuch)) {\r
+> +    fprintf (stderr, "Out of memory\n");\r
+> +    return 1;\r
+> +    }\r
+> +\r
+> +    if (notmuch_database_open (notmuch,\r
+> +                           notmuch_config_get_database_path (config),\r
+> +                           NOTMUCH_DATABASE_MODE_READ_ONLY))\r
+\r
+Does this need to destroy the database?  (Likewise for all the others.)\r
+\r
+>      return 1;\r
+>  \r
+>      query_str = query_string_from_args (config, argc-opt_index, argv+opt_index);\r
+> diff --git a/notmuch-dump.c b/notmuch-dump.c\r
+> index 2024e30..73579bc 100644\r
+> --- a/notmuch-dump.c\r
+> +++ b/notmuch-dump.c\r
+> @@ -33,8 +33,14 @@ notmuch_dump_command (notmuch_config_t *config, int argc, char *argv[])\r
+>      notmuch_tags_t *tags;\r
+>      const char *query_str = "";\r
+>  \r
+> -    if (notmuch_database_open (notmuch_config_get_database_path (config),\r
+> -                           NOTMUCH_DATABASE_MODE_READ_ONLY, &notmuch))\r
+> +    if (notmuch_database_new (&notmuch)) {\r
+> +    fprintf (stderr, "Out of memory\n");\r
+> +    return 1;\r
+> +    }\r
+> +\r
+> +    if (notmuch_database_open (notmuch,\r
+> +                           notmuch_config_get_database_path (config),\r
+> +                           NOTMUCH_DATABASE_MODE_READ_ONLY))\r
+>      return 1;\r
+>  \r
+>      char *output_file_name = NULL;\r
+> diff --git a/notmuch-insert.c b/notmuch-insert.c\r
+> index 2207b1e..4a8aad3 100644\r
+> --- a/notmuch-insert.c\r
+> +++ b/notmuch-insert.c\r
+> @@ -467,8 +467,14 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])\r
+>      action.sa_flags = 0;\r
+>      sigaction (SIGINT, &action, NULL);\r
+>  \r
+> -    if (notmuch_database_open (notmuch_config_get_database_path (config),\r
+> -                           NOTMUCH_DATABASE_MODE_READ_WRITE, &notmuch))\r
+> +    if (notmuch_database_new (&notmuch)) {\r
+> +    fprintf (stderr, "Out of memory\n");\r
+> +    return 1;\r
+> +    }\r
+> +\r
+> +    if (notmuch_database_open (notmuch,\r
+> +                           notmuch_config_get_database_path (config),\r
+> +                           NOTMUCH_DATABASE_MODE_READ_WRITE))\r
+>      return 1;\r
+>  \r
+>      ret = insert_message (config, notmuch, STDIN_FILENO, maildir, tag_ops);\r
+> diff --git a/notmuch-new.c b/notmuch-new.c\r
+> index ba05cb4..f72a4de 100644\r
+> --- a/notmuch-new.c\r
+> +++ b/notmuch-new.c\r
+> @@ -914,6 +914,11 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[])\r
+>          return ret;\r
+>      }\r
+>  \r
+> +    if (notmuch_database_new (&notmuch)) {\r
+> +    fprintf (stderr, "Out of memory\n");\r
+> +    return 1;\r
+> +    }\r
+> +\r
+>      dot_notmuch_path = talloc_asprintf (config, "%s/%s", db_path, ".notmuch");\r
+>  \r
+>      if (stat (dot_notmuch_path, &st)) {\r
+> @@ -925,12 +930,12 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[])\r
+>          return 1;\r
+>  \r
+>      printf ("Found %d total files (that's not much mail).\n", count);\r
+> -    if (notmuch_database_create (db_path, &notmuch))\r
+> +    if (notmuch_database_create (notmuch, db_path))\r
+>          return 1;\r
+>      add_files_state.total_files = count;\r
+>      } else {\r
+> -    if (notmuch_database_open (db_path, NOTMUCH_DATABASE_MODE_READ_WRITE,\r
+> -                               &notmuch))\r
+> +    if (notmuch_database_open (notmuch, db_path,\r
+> +                               NOTMUCH_DATABASE_MODE_READ_WRITE))\r
+>          return 1;\r
+>  \r
+>      if (notmuch_database_needs_upgrade (notmuch)) {\r
+> @@ -945,9 +950,6 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[])\r
+>      add_files_state.total_files = 0;\r
+>      }\r
+>  \r
+> -    if (notmuch == NULL)\r
+> -    return 1;\r
+> -\r
+>      /* Setup our handler for SIGINT. We do this after having\r
+>       * potentially done a database upgrade we this interrupt handler\r
+>       * won't support. */\r
+> diff --git a/notmuch-reply.c b/notmuch-reply.c\r
+> index 9d6f843..7b80841 100644\r
+> --- a/notmuch-reply.c\r
+> +++ b/notmuch-reply.c\r
+> @@ -769,8 +769,14 @@ notmuch_reply_command (notmuch_config_t *config, int argc, char *argv[])\r
+>      return 1;\r
+>      }\r
+>  \r
+> -    if (notmuch_database_open (notmuch_config_get_database_path (config),\r
+> -                           NOTMUCH_DATABASE_MODE_READ_ONLY, &notmuch))\r
+> +    if (notmuch_database_new (&notmuch)) {\r
+> +    fprintf (stderr, "Out of memory\n");\r
+> +    return 1;\r
+> +    }\r
+> +\r
+> +    if (notmuch_database_open (notmuch,\r
+> +                           notmuch_config_get_database_path (config),\r
+> +                           NOTMUCH_DATABASE_MODE_READ_ONLY))\r
+>      return 1;\r
+>  \r
+>      query = notmuch_query_create (notmuch, query_string);\r
+> diff --git a/notmuch-restore.c b/notmuch-restore.c\r
+> index 1419621..fc37838 100644\r
+> --- a/notmuch-restore.c\r
+> +++ b/notmuch-restore.c\r
+> @@ -138,8 +138,14 @@ notmuch_restore_command (notmuch_config_t *config, int argc, char *argv[])\r
+>      int opt_index;\r
+>      int input_format = DUMP_FORMAT_AUTO;\r
+>  \r
+> -    if (notmuch_database_open (notmuch_config_get_database_path (config),\r
+> -                           NOTMUCH_DATABASE_MODE_READ_WRITE, &notmuch))\r
+> +    if (notmuch_database_new (&notmuch)) {\r
+> +    fprintf (stderr, "Out of memory\n");\r
+> +    return 1;\r
+> +    }\r
+> +\r
+> +    if (notmuch_database_open (notmuch,\r
+> +                           notmuch_config_get_database_path (config),\r
+> +                           NOTMUCH_DATABASE_MODE_READ_WRITE))\r
+>      return 1;\r
+>  \r
+>      if (notmuch_config_get_maildir_synchronize_flags (config))\r
+> diff --git a/notmuch-search.c b/notmuch-search.c\r
+> index 7c973b3..50aace9 100644\r
+> --- a/notmuch-search.c\r
+> +++ b/notmuch-search.c\r
+> @@ -430,8 +430,14 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])\r
+>  \r
+>      notmuch_exit_if_unsupported_format ();\r
+>  \r
+> -    if (notmuch_database_open (notmuch_config_get_database_path (config),\r
+> -                           NOTMUCH_DATABASE_MODE_READ_ONLY, &notmuch))\r
+> +    if (notmuch_database_new (&notmuch)) {\r
+> +    fprintf (stderr, "Out of memory\n");\r
+> +    return 1;\r
+> +    }\r
+> +\r
+> +    if (notmuch_database_open (notmuch,\r
+> +                           notmuch_config_get_database_path (config),\r
+> +                           NOTMUCH_DATABASE_MODE_READ_ONLY))\r
+>      return 1;\r
+>  \r
+>      query_str = query_string_from_args (notmuch, argc-opt_index, argv+opt_index);\r
+> diff --git a/notmuch-show.c b/notmuch-show.c\r
+> index c07f887..bc44b2c 100644\r
+> --- a/notmuch-show.c\r
+> +++ b/notmuch-show.c\r
+> @@ -1201,8 +1201,14 @@ notmuch_show_command (notmuch_config_t *config, int argc, char *argv[])\r
+>      return 1;\r
+>      }\r
+>  \r
+> -    if (notmuch_database_open (notmuch_config_get_database_path (config),\r
+> -                           NOTMUCH_DATABASE_MODE_READ_ONLY, &notmuch))\r
+> +    if (notmuch_database_new (&notmuch)) {\r
+> +    fprintf (stderr, "Out of memory\n");\r
+> +    return 1;\r
+> +    }\r
+> +\r
+> +    if (notmuch_database_open (notmuch,\r
+> +                           notmuch_config_get_database_path (config),\r
+> +                           NOTMUCH_DATABASE_MODE_READ_ONLY))\r
+>      return 1;\r
+>  \r
+>      query = notmuch_query_create (notmuch, query_string);\r
+> diff --git a/notmuch-tag.c b/notmuch-tag.c\r
+> index 3b09df9..6e29799 100644\r
+> --- a/notmuch-tag.c\r
+> +++ b/notmuch-tag.c\r
+> @@ -254,8 +254,14 @@ notmuch_tag_command (notmuch_config_t *config, int argc, char *argv[])\r
+>      }\r
+>      }\r
+>  \r
+> -    if (notmuch_database_open (notmuch_config_get_database_path (config),\r
+> -                           NOTMUCH_DATABASE_MODE_READ_WRITE, &notmuch))\r
+> +    if (notmuch_database_new (&notmuch)) {\r
+> +    fprintf (stderr, "Out of memory\n");\r
+> +    return 1;\r
+> +    }\r
+> +\r
+> +    if (notmuch_database_open (notmuch,\r
+> +                           notmuch_config_get_database_path (config),\r
+> +                           NOTMUCH_DATABASE_MODE_READ_WRITE))\r
+>      return 1;\r
+>  \r
+>      if (notmuch_config_get_maildir_synchronize_flags (config))\r
+> diff --git a/test/random-corpus.c b/test/random-corpus.c\r
+> index 790193d..2b205e5 100644\r
+> --- a/test/random-corpus.c\r
+> +++ b/test/random-corpus.c\r
+> @@ -164,8 +164,14 @@ main (int argc, char **argv)\r
+>      if (config == NULL)\r
+>      return 1;\r
+>  \r
+> -    if (notmuch_database_open (notmuch_config_get_database_path (config),\r
+> -                           NOTMUCH_DATABASE_MODE_READ_WRITE, &notmuch))\r
+> +    if (notmuch_database_new (&notmuch)) {\r
+> +    fprintf (stderr, "Out of memory\n");\r
+> +    return 1;\r
+> +    }\r
+> +\r
+> +    if (notmuch_database_open (notmuch,\r
+> +                           notmuch_config_get_database_path (config),\r
+> +                           NOTMUCH_DATABASE_MODE_READ_WRITE))\r
+>      return 1;\r
+>  \r
+>      srandom (seed);\r
+> diff --git a/test/symbol-test.cc b/test/symbol-test.cc\r
+> index 3e96c03..47c5351 100644\r
+> --- a/test/symbol-test.cc\r
+> +++ b/test/symbol-test.cc\r
+> @@ -5,7 +5,8 @@\r
+>  \r
+>  int main() {\r
+>    notmuch_database_t *notmuch;\r
+> -  notmuch_database_open("fakedb", NOTMUCH_DATABASE_MODE_READ_ONLY, &notmuch);\r
+> +  notmuch_database_new (&notmuch);\r
+> +  notmuch_database_open (notmuch, "fakedb", NOTMUCH_DATABASE_MODE_READ_ONLY);\r
+>  \r
+>    try {\r
+>      (void) new Xapian::WritableDatabase("./nonexistant", Xapian::DB_OPEN);\r