a4/6560f778037588e9b0da21e18106f3cfc7074d

   1 Return-Path: <amdragon@mit.edu>\r
   2 X-Original-To: notmuch@notmuchmail.org\r
   3 Delivered-To: notmuch@notmuchmail.org\r
   4 Received: from localhost (localhost [127.0.0.1])\r
   5         by olra.theworths.org (Postfix) with ESMTP id 491BB431FD2\r
   6         for <notmuch@notmuchmail.org>; Wed,  4 Dec 2013 15:11:28 -0800 (PST)\r
   7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
   8 X-Spam-Flag: NO\r
   9 X-Spam-Score: -0.7\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5\r
  12         tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled\r
  13 Received: from olra.theworths.org ([127.0.0.1])\r
  14         by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
  15         with ESMTP id OryFt9l6vpw4 for <notmuch@notmuchmail.org>;\r
  16         Wed,  4 Dec 2013 15:11:23 -0800 (PST)\r
  17 Received: from dmz-mailsec-scanner-8.mit.edu (dmz-mailsec-scanner-8.mit.edu\r
  18         [18.7.68.37])\r
  19         (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits))\r
  20         (No client certificate requested)\r
  21         by olra.theworths.org (Postfix) with ESMTPS id 7A010431FD0\r
  22         for <notmuch@notmuchmail.org>; Wed,  4 Dec 2013 15:11:22 -0800 (PST)\r
  23 X-AuditID: 12074425-b7fd96d000000c39-7e-529fb698e984\r
  24 Received: from mailhub-auth-4.mit.edu ( [18.7.62.39])\r
  25         (using TLS with cipher AES256-SHA (256/256 bits))\r
  26         (Client did not present a certificate)\r
  27         by dmz-mailsec-scanner-8.mit.edu (Symantec Messaging Gateway) with SMTP\r
  28         id 8F.AE.03129.896BF925; Wed,  4 Dec 2013 18:11:20 -0500 (EST)\r
  29 Received: from outgoing.mit.edu (outgoing-auth-1.mit.edu [18.9.28.11])\r
  30         by mailhub-auth-4.mit.edu (8.13.8/8.9.2) with ESMTP id rB4NBJJ7032395; \r
  31         Wed, 4 Dec 2013 18:11:20 -0500\r
  32 Received: from awakening.csail.mit.edu (awakening.csail.mit.edu [18.26.4.91])\r
  33         (authenticated bits=0)\r
  34         (User authenticated as amdragon@ATHENA.MIT.EDU)\r
  35         by outgoing.mit.edu (8.13.8/8.12.4) with ESMTP id rB4NBGGu022234\r
  36         (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT);\r
  37         Wed, 4 Dec 2013 18:11:18 -0500\r
  38 Received: from amthrax by awakening.csail.mit.edu with local (Exim 4.80)\r
  39         (envelope-from <amdragon@mit.edu>)\r
  40         id 1VoLb7-0002im-TJ; Wed, 04 Dec 2013 18:11:15 -0500\r
  41 Date: Wed, 4 Dec 2013 18:11:13 -0500\r
  42 From: Austin Clements <amdragon@MIT.EDU>\r
  43 To: Jani Nikula <jani@nikula.org>\r
  44 Subject: Re: [PATCH 2/2] lib: introduce notmuch_database_new for initializing\r
  45         a database handle\r
  46 Message-ID: <20131204231113.GD8854@mit.edu>\r
  47 References: <cover.1385903109.git.jani@nikula.org>\r
  48         <fc7ecd990e55fcfba17de4d71e8823c98760f9ce.1385903109.git.jani@nikula.org>\r
  49 MIME-Version: 1.0\r
  50 Content-Type: text/plain; charset=us-ascii\r
  51 Content-Disposition: inline\r
  52 In-Reply-To:\r
  53  <fc7ecd990e55fcfba17de4d71e8823c98760f9ce.1385903109.git.jani@nikula.org>\r
  54 User-Agent: Mutt/1.5.21 (2010-09-15)\r
  55 X-Brightmail-Tracker:\r
  56  H4sIAAAAAAAAA+NgFmplleLIzCtJLcpLzFFi42IRYrdT152xbX6QwawPKhZN050trt+cyezA\r
  57         5HHr/mt2j2erbjEHMEVx2aSk5mSWpRbp2yVwZex5uImloPsIY8WEjuuMDYyPZzJ2MXJySAiY\r
  58         SLy9fw3KFpO4cG89WxcjF4eQwGwmic7PW5ghnA2MEr+71rBCOKeYJC6tvcAI4SxhlNjw/T8z\r
  59         SD+LgIpE/7ElbCA2m4CGxLb9y8HmiggoSmw+uR/MZhaQlvj2u5kJxBYWSJS4unkvWD2vgLbE\r
  60         /fZ2VhBbSKBOYt66fqi4oMTJmU9YIHq1JG78ewnUywE2Z/k/DpAwp0CYxI2OxewgtijQCVNO\r
  61         bmObwCg0C0n3LCTdsxC6FzAyr2KUTcmt0s1NzMwpTk3WLU5OzMtLLdK10MvNLNFLTSndxAgO\r
  62         bRfVHYwTDikdYhTgYFTi4XVImR8kxJpYVlyZe4hRkoNJSZTXZiNQiC8pP6UyI7E4I76oNCe1\r
  63         +BCjBAezkgjvvxqgHG9KYmVValE+TEqag0VJnPcWh32QkEB6YklqdmpqQWoRTFaGg0NJgvfH\r
  64         VqBGwaLU9NSKtMycEoQ0EwcnyHAeoOGXQWp4iwsSc4sz0yHypxgVpcR580ASAiCJjNI8uF5Y\r
  65         6nnFKA70ijDvbZAqHmDagut+BTSYCWhw84N5IINLEhFSUg2MzTu/Z7Zmthk8mjLZhFF+d7Pr\r
  66         1EPlktd5+ucGNt2qlssvWzbP8aWnQkbirMDm7k4PnTt35WO2aS4WLhDVvfZp3wHnD8EvDc8m\r
  67         54XaxLpKH93R5Pgn26Xsi1zNknMfroW0+lgt41plNiW+hC2O63Tlp/pnV4VW822vXsP7btuB\r
  68         z1d6Txs8FStRYinOSDTUYi4qTgQAf5Qg3RgDAAA=\r
  69 Cc: notmuch@notmuchmail.org\r
  70 X-BeenThere: notmuch@notmuchmail.org\r
  71 X-Mailman-Version: 2.1.13\r
  72 Precedence: list\r
  73 List-Id: "Use and development of the notmuch mail system."\r
  74         <notmuch.notmuchmail.org>\r
  75 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  76         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  77 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  78 List-Post: <mailto:notmuch@notmuchmail.org>\r
  79 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  80 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  81         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  82 X-List-Received-Date: Wed, 04 Dec 2013 23:11:28 -0000\r
  83 \r
  84 Quoth Jani Nikula on Dec 01 at  3:14 pm:\r
  85 > There is a need for setting options before opening a database, such as\r
  86 > setting a logging function to use instead of writing to stdout or\r
  87 > stderr. It would be possible to do this by adding new parameters to\r
  88 > notmuch_database_create and notmuch_database_open, but maintaining a\r
  89 > backwards compatible API and ABI when new options are added becomes\r
  90 > burdensome.\r
  91 > \r
  92 > Instead, split the opaque database object creation from\r
  93 > notmuch_database_create and notmuch_database_open into a new\r
  94 > notmuch_database_new call, to allow operations on the handle before\r
  95 > create and open. This creates API and ABI breakage now, but allows\r
  96 > easier future extensions.\r
  97 > \r
  98 > The notmuch_database_new call becomes a natural pair to the already\r
  99 > existing notmuch_database_destroy, and it should be possible to call\r
 100 > open/close multiple times using an initialized handle.\r
 101 \r
 102 A high-level comment about the API: Currently, an allocated\r
 103 notmuch_database_t has two "memory states", if you will: open and\r
 104 closed.  (I wish it didn't have any memory states, and was on the\r
 105 fence about this API for a while until I realized that the ship had\r
 106 already sailed.)  It's pretty clear how all of the notmuch APIs will\r
 107 behave in both states (modulo some bounded non-determinism in the\r
 108 closed state).  I think this patch introduces a new "pre-open" state,\r
 109 and I don't know how most of the notmuch APIs behave in that state.\r
 110 My guess is poorly.  If it's feasible, I'd much rather a fresh baked\r
 111 notmuch_database_t act like it's in the closed state, including that\r
 112 notmuch_database_{create,open} are well-defined as transitions from\r
 113 closed state to open state (even if the closed state was reached by\r
 114 calling notmuch_database_close).  Or, if we do have a "pre-open"\r
 115 state, it should at least be well-specified what that means\r
 116 (preferably the specification is *not* "most APIs segfault").\r
 117 \r
 118 Orthogonally -- and this may be a complete pipe dream of mine -- if we\r
 119 just had a way to return more detailed error information than a simple\r
 120 error code from notmuch_database_{create,open}, I think we wouldn't\r
 121 need any of this.  Everything that these functions currently log\r
 122 (modulo one warning) is error details, so if we could return the error\r
 123 details *with the error* or somehow make them accessible, we wouldn't\r
 124 need a logger at this point (or at several other points in the\r
 125 library).\r
 126 \r
 127 > ---\r
 128 >  lib/database.cc      | 64 ++++++++++++++++++++++++++++------------------------\r
 129 >  lib/notmuch.h        | 52 ++++++++++++++++++++++++++++++++----------\r
 130 >  notmuch-compact.c    | 11 ++++++++-\r
 131 >  notmuch-count.c      | 10 ++++++--\r
 132 >  notmuch-dump.c       | 10 ++++++--\r
 133 >  notmuch-insert.c     | 10 ++++++--\r
 134 >  notmuch-new.c        | 14 +++++++-----\r
 135 >  notmuch-reply.c      | 10 ++++++--\r
 136 >  notmuch-restore.c    | 10 ++++++--\r
 137 >  notmuch-search.c     | 10 ++++++--\r
 138 >  notmuch-show.c       | 10 ++++++--\r
 139 >  notmuch-tag.c        | 10 ++++++--\r
 140 >  test/random-corpus.c | 10 ++++++--\r
 141 >  test/symbol-test.cc  |  3 ++-\r
 142 >  14 files changed, 166 insertions(+), 68 deletions(-)\r
 143 > \r
 144 > diff --git a/lib/database.cc b/lib/database.cc\r
 145 > index 98e2c31..386b93a 100644\r
 146 > --- a/lib/database.cc\r
 147 > +++ b/lib/database.cc\r
 148 > @@ -539,10 +539,21 @@ parse_references (void *ctx,\r
 149 >  }\r
 150 >  \r
 151 >  notmuch_status_t\r
 152 > -notmuch_database_create (const char *path, notmuch_database_t **database)\r
 153 > +notmuch_database_new (notmuch_database_t **notmuch)\r
 154 \r
 155 The naming of this is unfortunate...  Other APIs use x_create to\r
 156 allocate objects (e.g., notmuch_query_create, several internal APIs).\r
 157 I would lean towards calling this function notmuch_database_create,\r
 158 but that leaves the question of what to call the other.  While we're\r
 159 breaking APIs, would it be completely crazy to merge open and create\r
 160 into one API with an extra mode to indicate creation (it can be its\r
 161 own mode because creation implies read/write)?  (Or, in UNIX\r
 162 tradition, we could call this function notmuch_database_create and the\r
 163 other notmuch_database_creat.)  notmuch_database_create is already\r
 164 just a shell around notmuch_database_open (we could keep it as a\r
 165 separate function, but just make it internal).\r
 166 \r
 167 > +{\r
 168 > +    /* Note: try to avoid error conditions! No error printing! */\r
 169 > +\r
 170 > +    *notmuch = talloc_zero (NULL, notmuch_database_t);\r
 171 > +    if (! *notmuch)\r
 172 > +     return NOTMUCH_STATUS_OUT_OF_MEMORY;\r
 173 > +\r
 174 > +    return NOTMUCH_STATUS_SUCCESS;\r
 175 > +}\r
 176 > +\r
 177 > +notmuch_status_t\r
 178 > +notmuch_database_create (notmuch_database_t *notmuch, const char *path)\r
 179 >  {\r
 180 \r
 181 This should fail if passed a database that is already open.\r
 182 \r
 183 >      notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;\r
 184 > -    notmuch_database_t *notmuch = NULL;\r
 185 >      char *notmuch_path = NULL;\r
 186 >      struct stat st;\r
 187 >      int err;\r
 188 > @@ -579,25 +590,18 @@ notmuch_database_create (const char *path, notmuch_database_t **database)\r
 189 >       goto DONE;\r
 190 >      }\r
 191 >  \r
 192 > -    status = notmuch_database_open (path,\r
 193 > -                                 NOTMUCH_DATABASE_MODE_READ_WRITE,\r
 194 > -                                 &notmuch);\r
 195 > +    status = notmuch_database_open (notmuch, path,\r
 196 > +                                 NOTMUCH_DATABASE_MODE_READ_WRITE);\r
 197 >      if (status)\r
 198 >       goto DONE;\r
 199 >      status = notmuch_database_upgrade (notmuch, NULL, NULL);\r
 200 > -    if (status) {\r
 201 > +    if (status)\r
 202 >       notmuch_database_close(notmuch);\r
 203 > -     notmuch = NULL;\r
 204 > -    }\r
 205 >  \r
 206 >    DONE:\r
 207 >      if (notmuch_path)\r
 208 >       talloc_free (notmuch_path);\r
 209 >  \r
 210 > -    if (database)\r
 211 > -     *database = notmuch;\r
 212 > -    else\r
 213 > -     talloc_free (notmuch);\r
 214 >      return status;\r
 215 >  }\r
 216 >  \r
 217 > @@ -612,14 +616,15 @@ _notmuch_database_ensure_writable (notmuch_database_t *notmuch)\r
 218 >      return NOTMUCH_STATUS_SUCCESS;\r
 219 >  }\r
 220 >  \r
 221 > +/*\r
 222 > + * XXX: error handling should clean up *all* state created!\r
 223 > + */\r
 224 \r
 225 I think the only thing that will currently leak from this in an error\r
 226 case is notmuch->path.\r
 227 \r
 228 >  notmuch_status_t\r
 229 > -notmuch_database_open (const char *path,\r
 230 > -                    notmuch_database_mode_t mode,\r
 231 > -                    notmuch_database_t **database)\r
 232 > +notmuch_database_open (notmuch_database_t *notmuch, const char *path,\r
 233 > +                    notmuch_database_mode_t mode)\r
 234 >  {\r
 235 \r
 236 This should also fail if passed a database that is already open.\r
 237 \r
 238 >      notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;\r
 239 >      void *local = talloc_new (NULL);\r
 240 > -    notmuch_database_t *notmuch = NULL;\r
 241 >      char *notmuch_path, *xapian_path;\r
 242 >      struct stat st;\r
 243 >      int err;\r
 244 > @@ -663,7 +668,6 @@ notmuch_database_open (const char *path,\r
 245 >       initialized = 1;\r
 246 >      }\r
 247 >  \r
 248 > -    notmuch = talloc_zero (NULL, notmuch_database_t);\r
 249 >      notmuch->exception_reported = FALSE;\r
 250 >      notmuch->path = talloc_strdup (notmuch, path);\r
 251 >  \r
 252 > @@ -689,8 +693,7 @@ notmuch_database_open (const char *path,\r
 253 >                        "       read-write mode.\n",\r
 254 >                        notmuch_path, version, NOTMUCH_DATABASE_VERSION);\r
 255 >               notmuch->mode = NOTMUCH_DATABASE_MODE_READ_ONLY;\r
 256 > -             notmuch_database_destroy (notmuch);\r
 257 > -             notmuch = NULL;\r
 258 > +             notmuch_database_close (notmuch);\r
 259 >               status = NOTMUCH_STATUS_FILE_ERROR;\r
 260 >               goto DONE;\r
 261 >           }\r
 262 > @@ -752,21 +755,19 @@ notmuch_database_open (const char *path,\r
 263 >      } catch (const Xapian::Error &error) {\r
 264 >       fprintf (stderr, "A Xapian exception occurred opening database: %s\n",\r
 265 >                error.get_msg().c_str());\r
 266 > -     notmuch_database_destroy (notmuch);\r
 267 > -     notmuch = NULL;\r
 268 > +     notmuch_database_close (notmuch);\r
 269 >       status = NOTMUCH_STATUS_XAPIAN_EXCEPTION;\r
 270 >      }\r
 271 >  \r
 272 >    DONE:\r
 273 >      talloc_free (local);\r
 274 >  \r
 275 \r
 276 It might be simpler to call notmuch_database_close here if status !=\r
 277 NOTMUCH_STATUS_SUCCESS, rather than calling it at several places above\r
 278 (and not on all error paths).\r
 279 \r
 280 > -    if (database)\r
 281 > -     *database = notmuch;\r
 282 > -    else\r
 283 > -     talloc_free (notmuch);\r
 284 >      return status;\r
 285 >  }\r
 286 >  \r
 287 > +/*\r
 288 > + * XXX: close should clean up *all* state created by open/create!\r
 289 > + */\r
 290 \r
 291 I believe the only thing it doesn't clean up is path.  (Note that\r
 292 cleaning up path here doesn't currently negate the need to clean up\r
 293 path above, though if you float the close call to the DONE path, it\r
 294 would suffice.)\r
 295 \r
 296 >  notmuch_status_t\r
 297 >  notmuch_database_close (notmuch_database_t *notmuch)\r
 298 >  {\r
 299 > @@ -869,7 +870,8 @@ public:\r
 300 >   * compaction process to protect data integrity.\r
 301 >   */\r
 302 >  notmuch_status_t\r
 303 > -notmuch_database_compact (const char *path,\r
 304 > +notmuch_database_compact (notmuch_database_t *notmuch,\r
 305 > +                       const char *path,\r
 306 >                         const char *backup_path,\r
 307 >                         notmuch_compact_status_cb_t status_cb,\r
 308 >                         void *closure)\r
 309 > @@ -877,7 +879,6 @@ notmuch_database_compact (const char *path,\r
 310 >      void *local;\r
 311 >      char *notmuch_path, *xapian_path, *compact_xapian_path;\r
 312 >      notmuch_status_t ret = NOTMUCH_STATUS_SUCCESS;\r
 313 > -    notmuch_database_t *notmuch = NULL;\r
 314 >      struct stat statbuf;\r
 315 >      notmuch_bool_t keep_backup;\r
 316 >  \r
 317 > @@ -885,7 +886,8 @@ notmuch_database_compact (const char *path,\r
 318 >      if (! local)\r
 319 >       return NOTMUCH_STATUS_OUT_OF_MEMORY;\r
 320 >  \r
 321 > -    ret = notmuch_database_open (path, NOTMUCH_DATABASE_MODE_READ_WRITE, &notmuch);\r
 322 > +    ret = notmuch_database_open (notmuch, path,\r
 323 > +                              NOTMUCH_DATABASE_MODE_READ_WRITE);\r
 324 >      if (ret) {\r
 325 >       goto DONE;\r
 326 >      }\r
 327 > @@ -971,8 +973,9 @@ notmuch_database_compact (const char *path,\r
 328 >      }\r
 329 >  \r
 330 >    DONE:\r
 331 > +    /* XXX: error handling */\r
 332 >      if (notmuch)\r
 333 > -     ret = notmuch_database_destroy (notmuch);\r
 334 > +     ret = notmuch_database_close (notmuch);\r
 335 >  \r
 336 >      talloc_free (local);\r
 337 >  \r
 338 > @@ -980,7 +983,8 @@ notmuch_database_compact (const char *path,\r
 339 >  }\r
 340 >  #else\r
 341 >  notmuch_status_t\r
 342 > -notmuch_database_compact (unused (const char *path),\r
 343 > +notmuch_database_compact (unused (notmuch_database_t *notmuch),\r
 344 > +                       unused (const char *path),\r
 345 >                         unused (const char *backup_path),\r
 346 >                         unused (notmuch_compact_status_cb_t status_cb),\r
 347 >                         unused (void *closure))\r
 348 > diff --git a/lib/notmuch.h b/lib/notmuch.h\r
 349 > index dbdce86..cd58d15 100644\r
 350 > --- a/lib/notmuch.h\r
 351 > +++ b/lib/notmuch.h\r
 352 > @@ -149,6 +149,28 @@ typedef struct _notmuch_tags notmuch_tags_t;\r
 353 >  typedef struct _notmuch_directory notmuch_directory_t;\r
 354 >  typedef struct _notmuch_filenames notmuch_filenames_t;\r
 355 >  \r
 356 > +/* Initialize a new, empty database handle.\r
 357 > + *\r
 358 > + * The database handle is required for creating, opening, and\r
 359 > + * compacting a database. For further database operations, the\r
 360 > + * database needs to be created or opened.\r
 361 > + *\r
 362 > + * After a successful call to notmuch_database_new, the returned\r
 363 > + * database handle will remain in memory, so the caller should call\r
 364 > + * notmuch_database_destroy when finished with the database handle.\r
 365 > + *\r
 366 > + * In case of any failure, this function returns an error status and\r
 367 > + * sets *notmuch to NULL.\r
 368 > + *\r
 369 > + * Return value:\r
 370 > + *\r
 371 > + * NOTMUCH_STATUS_SUCCESS: Successfully created the database object.\r
 372 > + *\r
 373 > + * NOTMUCH_STATUS_OUT_OF_MEMORY: Out of memory.\r
 374 > + */\r
 375 > +notmuch_status_t\r
 376 > +notmuch_database_new (notmuch_database_t **notmuch);\r
 377 > +\r
 378 >  /* Create a new, empty notmuch database located at 'path'.\r
 379 >   *\r
 380 >   * The path should be a top-level directory to a collection of\r
 381 > @@ -156,9 +178,9 @@ typedef struct _notmuch_filenames notmuch_filenames_t;\r
 382 >   * create a new ".notmuch" directory within 'path' where notmuch will\r
 383 >   * store its data.\r
 384 >   *\r
 385 > - * After a successful call to notmuch_database_create, the returned\r
 386 > - * database will be open so the caller should call\r
 387 > - * notmuch_database_destroy when finished with it.\r
 388 > + * After a successful call to notmuch_database_create, the database\r
 389 > + * will be open so the caller should call notmuch_database_close (or\r
 390 > + * notmuch_database_destroy) when finished with the database.\r
 391 >   *\r
 392 >   * The database will not yet have any data in it\r
 393 >   * (notmuch_database_create itself is a very cheap function). Messages\r
 394 > @@ -166,7 +188,8 @@ typedef struct _notmuch_filenames notmuch_filenames_t;\r
 395 >   * notmuch_database_add_message.\r
 396 >   *\r
 397 >   * In case of any failure, this function returns an error status and\r
 398 > - * sets *database to NULL (after printing an error message on stderr).\r
 399 > + * the database will be closed (after printing an error message on\r
 400 > + * stderr).\r
 401 >   *\r
 402 >   * Return value:\r
 403 >   *\r
 404 > @@ -183,7 +206,7 @@ typedef struct _notmuch_filenames notmuch_filenames_t;\r
 405 >   * NOTMUCH_STATUS_XAPIAN_EXCEPTION: A Xapian exception occurred.\r
 406 >   */\r
 407 >  notmuch_status_t\r
 408 > -notmuch_database_create (const char *path, notmuch_database_t **database);\r
 409 > +notmuch_database_create (notmuch_database_t *notmuch, const char *path);\r
 410 >  \r
 411 >  typedef enum {\r
 412 >      NOTMUCH_DATABASE_MODE_READ_ONLY = 0,\r
 413 > @@ -201,11 +224,13 @@ typedef enum {\r
 414 >   * An existing notmuch database can be identified by the presence of a\r
 415 >   * directory named ".notmuch" below 'path'.\r
 416 >   *\r
 417 > - * The caller should call notmuch_database_destroy when finished with\r
 418 > - * this database.\r
 419 > + * After a successful call to notmuch_database_open, the database will\r
 420 > + * be open so the caller should call notmuch_database_close (or\r
 421 > + * notmuch_database_destroy) when finished with the database.\r
 422 >   *\r
 423 >   * In case of any failure, this function returns an error status and\r
 424 > - * sets *database to NULL (after printing an error message on stderr).\r
 425 > + * the database will be closed (after printing an error message on\r
 426 > + * stderr).\r
 427 >   *\r
 428 >   * Return value:\r
 429 >   *\r
 430 > @@ -222,9 +247,8 @@ typedef enum {\r
 431 >   * NOTMUCH_STATUS_XAPIAN_EXCEPTION: A Xapian exception occurred.\r
 432 >   */\r
 433 >  notmuch_status_t\r
 434 > -notmuch_database_open (const char *path,\r
 435 > -                    notmuch_database_mode_t mode,\r
 436 > -                    notmuch_database_t **database);\r
 437 > +notmuch_database_open (notmuch_database_t *notmuch, const char *path,\r
 438 > +                    notmuch_database_mode_t mode);\r
 439 >  \r
 440 >  /* Close the given notmuch database.\r
 441 >   *\r
 442 > @@ -264,7 +288,8 @@ typedef void (*notmuch_compact_status_cb_t)(const char *message, void *closure);\r
 443 >   * 'closure' is passed verbatim to any callback invoked.\r
 444 >   */\r
 445 >  notmuch_status_t\r
 446 > -notmuch_database_compact (const char* path,\r
 447 > +notmuch_database_compact (notmuch_database_t *notmuch,\r
 448 > +                       const char* path,\r
 449 >                         const char* backup_path,\r
 450 >                         notmuch_compact_status_cb_t status_cb,\r
 451 >                         void *closure);\r
 452 > @@ -272,6 +297,9 @@ notmuch_database_compact (const char* path,\r
 453 >  /* Destroy the notmuch database, closing it if necessary and freeing\r
 454 >   * all associated resources.\r
 455 >   *\r
 456 > + * A database handle initialized with notmuch_database_new should be\r
 457 > + * destroyed by calling notmuch_database_destroy.\r
 458 > + *\r
 459 >   * Return value as in notmuch_database_close if the database was open;\r
 460 >   * notmuch_database_destroy itself has no failure modes.\r
 461 >   */\r
 462 > diff --git a/notmuch-compact.c b/notmuch-compact.c\r
 463 > index 8b820c0..626685e 100644\r
 464 > --- a/notmuch-compact.c\r
 465 > +++ b/notmuch-compact.c\r
 466 > @@ -29,6 +29,7 @@ status_update_cb (const char *msg, unused (void *closure))\r
 467 >  int\r
 468 >  notmuch_compact_command (notmuch_config_t *config, int argc, char *argv[])\r
 469 >  {\r
 470 > +    notmuch_database_t *notmuch = NULL;\r
 471 >      const char *path = notmuch_config_get_database_path (config);\r
 472 >      const char *backup_path = NULL;\r
 473 >      notmuch_status_t ret;\r
 474 > @@ -46,7 +47,13 @@ notmuch_compact_command (notmuch_config_t *config, int argc, char *argv[])\r
 475 >  \r
 476 >      if (! quiet)\r
 477 >       printf ("Compacting database...\n");\r
 478 > -    ret = notmuch_database_compact (path, backup_path,\r
 479 > +\r
 480 > +    if (notmuch_database_new (&notmuch)) {\r
 481 > +     fprintf (stderr, "Out of memory\n");\r
 482 > +     return 1;\r
 483 > +    }\r
 484 > +\r
 485 > +    ret = notmuch_database_compact (notmuch, path, backup_path,\r
 486 >                                   quiet ? NULL : status_update_cb, NULL);\r
 487 >      if (ret) {\r
 488 >       fprintf (stderr, "Compaction failed: %s\n", notmuch_status_to_string (ret));\r
 489 > @@ -60,5 +67,7 @@ notmuch_compact_command (notmuch_config_t *config, int argc, char *argv[])\r
 490 >       printf ("Done.\n");\r
 491 >      }\r
 492 >  \r
 493 > +    notmuch_database_destroy (notmuch);\r
 494 > +\r
 495 >      return 0;\r
 496 >  }\r
 497 > diff --git a/notmuch-count.c b/notmuch-count.c\r
 498 > index 01e4e30..15c95c7 100644\r
 499 > --- a/notmuch-count.c\r
 500 > +++ b/notmuch-count.c\r
 501 > @@ -170,8 +170,14 @@ notmuch_count_command (notmuch_config_t *config, int argc, char *argv[])\r
 502 >       return 1;\r
 503 >      }\r
 504 >  \r
 505 > -    if (notmuch_database_open (notmuch_config_get_database_path (config),\r
 506 > -                            NOTMUCH_DATABASE_MODE_READ_ONLY, &notmuch))\r
 507 > +    if (notmuch_database_new (&notmuch)) {\r
 508 > +     fprintf (stderr, "Out of memory\n");\r
 509 > +     return 1;\r
 510 > +    }\r
 511 > +\r
 512 > +    if (notmuch_database_open (notmuch,\r
 513 > +                            notmuch_config_get_database_path (config),\r
 514 > +                            NOTMUCH_DATABASE_MODE_READ_ONLY))\r
 515 \r
 516 Does this need to destroy the database?  (Likewise for all the others.)\r
 517 \r
 518 >       return 1;\r
 519 >  \r
 520 >      query_str = query_string_from_args (config, argc-opt_index, argv+opt_index);\r
 521 > diff --git a/notmuch-dump.c b/notmuch-dump.c\r
 522 > index 2024e30..73579bc 100644\r
 523 > --- a/notmuch-dump.c\r
 524 > +++ b/notmuch-dump.c\r
 525 > @@ -33,8 +33,14 @@ notmuch_dump_command (notmuch_config_t *config, int argc, char *argv[])\r
 526 >      notmuch_tags_t *tags;\r
 527 >      const char *query_str = "";\r
 528 >  \r
 529 > -    if (notmuch_database_open (notmuch_config_get_database_path (config),\r
 530 > -                            NOTMUCH_DATABASE_MODE_READ_ONLY, &notmuch))\r
 531 > +    if (notmuch_database_new (&notmuch)) {\r
 532 > +     fprintf (stderr, "Out of memory\n");\r
 533 > +     return 1;\r
 534 > +    }\r
 535 > +\r
 536 > +    if (notmuch_database_open (notmuch,\r
 537 > +                            notmuch_config_get_database_path (config),\r
 538 > +                            NOTMUCH_DATABASE_MODE_READ_ONLY))\r
 539 >       return 1;\r
 540 >  \r
 541 >      char *output_file_name = NULL;\r
 542 > diff --git a/notmuch-insert.c b/notmuch-insert.c\r
 543 > index 2207b1e..4a8aad3 100644\r
 544 > --- a/notmuch-insert.c\r
 545 > +++ b/notmuch-insert.c\r
 546 > @@ -467,8 +467,14 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])\r
 547 >      action.sa_flags = 0;\r
 548 >      sigaction (SIGINT, &action, NULL);\r
 549 >  \r
 550 > -    if (notmuch_database_open (notmuch_config_get_database_path (config),\r
 551 > -                            NOTMUCH_DATABASE_MODE_READ_WRITE, &notmuch))\r
 552 > +    if (notmuch_database_new (&notmuch)) {\r
 553 > +     fprintf (stderr, "Out of memory\n");\r
 554 > +     return 1;\r
 555 > +    }\r
 556 > +\r
 557 > +    if (notmuch_database_open (notmuch,\r
 558 > +                            notmuch_config_get_database_path (config),\r
 559 > +                            NOTMUCH_DATABASE_MODE_READ_WRITE))\r
 560 >       return 1;\r
 561 >  \r
 562 >      ret = insert_message (config, notmuch, STDIN_FILENO, maildir, tag_ops);\r
 563 > diff --git a/notmuch-new.c b/notmuch-new.c\r
 564 > index ba05cb4..f72a4de 100644\r
 565 > --- a/notmuch-new.c\r
 566 > +++ b/notmuch-new.c\r
 567 > @@ -914,6 +914,11 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[])\r
 568 >           return ret;\r
 569 >      }\r
 570 >  \r
 571 > +    if (notmuch_database_new (&notmuch)) {\r
 572 > +     fprintf (stderr, "Out of memory\n");\r
 573 > +     return 1;\r
 574 > +    }\r
 575 > +\r
 576 >      dot_notmuch_path = talloc_asprintf (config, "%s/%s", db_path, ".notmuch");\r
 577 >  \r
 578 >      if (stat (dot_notmuch_path, &st)) {\r
 579 > @@ -925,12 +930,12 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[])\r
 580 >           return 1;\r
 581 >  \r
 582 >       printf ("Found %d total files (that's not much mail).\n", count);\r
 583 > -     if (notmuch_database_create (db_path, &notmuch))\r
 584 > +     if (notmuch_database_create (notmuch, db_path))\r
 585 >           return 1;\r
 586 >       add_files_state.total_files = count;\r
 587 >      } else {\r
 588 > -     if (notmuch_database_open (db_path, NOTMUCH_DATABASE_MODE_READ_WRITE,\r
 589 > -                                &notmuch))\r
 590 > +     if (notmuch_database_open (notmuch, db_path,\r
 591 > +                                NOTMUCH_DATABASE_MODE_READ_WRITE))\r
 592 >           return 1;\r
 593 >  \r
 594 >       if (notmuch_database_needs_upgrade (notmuch)) {\r
 595 > @@ -945,9 +950,6 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[])\r
 596 >       add_files_state.total_files = 0;\r
 597 >      }\r
 598 >  \r
 599 > -    if (notmuch == NULL)\r
 600 > -     return 1;\r
 601 > -\r
 602 >      /* Setup our handler for SIGINT. We do this after having\r
 603 >       * potentially done a database upgrade we this interrupt handler\r
 604 >       * won't support. */\r
 605 > diff --git a/notmuch-reply.c b/notmuch-reply.c\r
 606 > index 9d6f843..7b80841 100644\r
 607 > --- a/notmuch-reply.c\r
 608 > +++ b/notmuch-reply.c\r
 609 > @@ -769,8 +769,14 @@ notmuch_reply_command (notmuch_config_t *config, int argc, char *argv[])\r
 610 >       return 1;\r
 611 >      }\r
 612 >  \r
 613 > -    if (notmuch_database_open (notmuch_config_get_database_path (config),\r
 614 > -                            NOTMUCH_DATABASE_MODE_READ_ONLY, &notmuch))\r
 615 > +    if (notmuch_database_new (&notmuch)) {\r
 616 > +     fprintf (stderr, "Out of memory\n");\r
 617 > +     return 1;\r
 618 > +    }\r
 619 > +\r
 620 > +    if (notmuch_database_open (notmuch,\r
 621 > +                            notmuch_config_get_database_path (config),\r
 622 > +                            NOTMUCH_DATABASE_MODE_READ_ONLY))\r
 623 >       return 1;\r
 624 >  \r
 625 >      query = notmuch_query_create (notmuch, query_string);\r
 626 > diff --git a/notmuch-restore.c b/notmuch-restore.c\r
 627 > index 1419621..fc37838 100644\r
 628 > --- a/notmuch-restore.c\r
 629 > +++ b/notmuch-restore.c\r
 630 > @@ -138,8 +138,14 @@ notmuch_restore_command (notmuch_config_t *config, int argc, char *argv[])\r
 631 >      int opt_index;\r
 632 >      int input_format = DUMP_FORMAT_AUTO;\r
 633 >  \r
 634 > -    if (notmuch_database_open (notmuch_config_get_database_path (config),\r
 635 > -                            NOTMUCH_DATABASE_MODE_READ_WRITE, &notmuch))\r
 636 > +    if (notmuch_database_new (&notmuch)) {\r
 637 > +     fprintf (stderr, "Out of memory\n");\r
 638 > +     return 1;\r
 639 > +    }\r
 640 > +\r
 641 > +    if (notmuch_database_open (notmuch,\r
 642 > +                            notmuch_config_get_database_path (config),\r
 643 > +                            NOTMUCH_DATABASE_MODE_READ_WRITE))\r
 644 >       return 1;\r
 645 >  \r
 646 >      if (notmuch_config_get_maildir_synchronize_flags (config))\r
 647 > diff --git a/notmuch-search.c b/notmuch-search.c\r
 648 > index 7c973b3..50aace9 100644\r
 649 > --- a/notmuch-search.c\r
 650 > +++ b/notmuch-search.c\r
 651 > @@ -430,8 +430,14 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])\r
 652 >  \r
 653 >      notmuch_exit_if_unsupported_format ();\r
 654 >  \r
 655 > -    if (notmuch_database_open (notmuch_config_get_database_path (config),\r
 656 > -                            NOTMUCH_DATABASE_MODE_READ_ONLY, &notmuch))\r
 657 > +    if (notmuch_database_new (&notmuch)) {\r
 658 > +     fprintf (stderr, "Out of memory\n");\r
 659 > +     return 1;\r
 660 > +    }\r
 661 > +\r
 662 > +    if (notmuch_database_open (notmuch,\r
 663 > +                            notmuch_config_get_database_path (config),\r
 664 > +                            NOTMUCH_DATABASE_MODE_READ_ONLY))\r
 665 >       return 1;\r
 666 >  \r
 667 >      query_str = query_string_from_args (notmuch, argc-opt_index, argv+opt_index);\r
 668 > diff --git a/notmuch-show.c b/notmuch-show.c\r
 669 > index c07f887..bc44b2c 100644\r
 670 > --- a/notmuch-show.c\r
 671 > +++ b/notmuch-show.c\r
 672 > @@ -1201,8 +1201,14 @@ notmuch_show_command (notmuch_config_t *config, int argc, char *argv[])\r
 673 >       return 1;\r
 674 >      }\r
 675 >  \r
 676 > -    if (notmuch_database_open (notmuch_config_get_database_path (config),\r
 677 > -                            NOTMUCH_DATABASE_MODE_READ_ONLY, &notmuch))\r
 678 > +    if (notmuch_database_new (&notmuch)) {\r
 679 > +     fprintf (stderr, "Out of memory\n");\r
 680 > +     return 1;\r
 681 > +    }\r
 682 > +\r
 683 > +    if (notmuch_database_open (notmuch,\r
 684 > +                            notmuch_config_get_database_path (config),\r
 685 > +                            NOTMUCH_DATABASE_MODE_READ_ONLY))\r
 686 >       return 1;\r
 687 >  \r
 688 >      query = notmuch_query_create (notmuch, query_string);\r
 689 > diff --git a/notmuch-tag.c b/notmuch-tag.c\r
 690 > index 3b09df9..6e29799 100644\r
 691 > --- a/notmuch-tag.c\r
 692 > +++ b/notmuch-tag.c\r
 693 > @@ -254,8 +254,14 @@ notmuch_tag_command (notmuch_config_t *config, int argc, char *argv[])\r
 694 >       }\r
 695 >      }\r
 696 >  \r
 697 > -    if (notmuch_database_open (notmuch_config_get_database_path (config),\r
 698 > -                            NOTMUCH_DATABASE_MODE_READ_WRITE, &notmuch))\r
 699 > +    if (notmuch_database_new (&notmuch)) {\r
 700 > +     fprintf (stderr, "Out of memory\n");\r
 701 > +     return 1;\r
 702 > +    }\r
 703 > +\r
 704 > +    if (notmuch_database_open (notmuch,\r
 705 > +                            notmuch_config_get_database_path (config),\r
 706 > +                            NOTMUCH_DATABASE_MODE_READ_WRITE))\r
 707 >       return 1;\r
 708 >  \r
 709 >      if (notmuch_config_get_maildir_synchronize_flags (config))\r
 710 > diff --git a/test/random-corpus.c b/test/random-corpus.c\r
 711 > index 790193d..2b205e5 100644\r
 712 > --- a/test/random-corpus.c\r
 713 > +++ b/test/random-corpus.c\r
 714 > @@ -164,8 +164,14 @@ main (int argc, char **argv)\r
 715 >      if (config == NULL)\r
 716 >       return 1;\r
 717 >  \r
 718 > -    if (notmuch_database_open (notmuch_config_get_database_path (config),\r
 719 > -                            NOTMUCH_DATABASE_MODE_READ_WRITE, &notmuch))\r
 720 > +    if (notmuch_database_new (&notmuch)) {\r
 721 > +     fprintf (stderr, "Out of memory\n");\r
 722 > +     return 1;\r
 723 > +    }\r
 724 > +\r
 725 > +    if (notmuch_database_open (notmuch,\r
 726 > +                            notmuch_config_get_database_path (config),\r
 727 > +                            NOTMUCH_DATABASE_MODE_READ_WRITE))\r
 728 >       return 1;\r
 729 >  \r
 730 >      srandom (seed);\r
 731 > diff --git a/test/symbol-test.cc b/test/symbol-test.cc\r
 732 > index 3e96c03..47c5351 100644\r
 733 > --- a/test/symbol-test.cc\r
 734 > +++ b/test/symbol-test.cc\r
 735 > @@ -5,7 +5,8 @@\r
 736 >  \r
 737 >  int main() {\r
 738 >    notmuch_database_t *notmuch;\r
 739 > -  notmuch_database_open("fakedb", NOTMUCH_DATABASE_MODE_READ_ONLY, &notmuch);\r
 740 > +  notmuch_database_new (&notmuch);\r
 741 > +  notmuch_database_open (notmuch, "fakedb", NOTMUCH_DATABASE_MODE_READ_ONLY);\r
 742 >  \r
 743 >    try {\r
 744 >      (void) new Xapian::WritableDatabase("./nonexistant", Xapian::DB_OPEN);\r