[PATCH 1/7] Split notmuch_database_close into two functions
Austin Clements
amdragon at MIT.EDU
Sun Apr 22 11:01:59 PDT 2012
Quoth Justus Winter on Apr 22 at 2:07 pm:
> Formerly notmuch_database_close closed the xapian database and
> destroyed the talloc structure associated with the notmuch database
> object. Split notmuch_database_close into notmuch_database_close and
> notmuch_database_destroy.
>
> This makes it possible for long running programs to close the xapian
> database and thus release the lock associated with it without
> destroying the data structures obtained from it.
>
> This also makes the api more consistent since every other data
> structure has a destructor function.
>
> Signed-off-by: Justus Winter <4winter at informatik.uni-hamburg.de>
> ---
> lib/database.cc | 14 ++++++++++++--
> lib/notmuch.h | 15 +++++++++++----
> 2 files changed, 23 insertions(+), 6 deletions(-)
>
> diff --git a/lib/database.cc b/lib/database.cc
> index 16c4354..2fefcad 100644
> --- a/lib/database.cc
> +++ b/lib/database.cc
> @@ -642,7 +642,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_close (notmuch);
> + notmuch_database_destroy (notmuch);
> notmuch = NULL;
> goto DONE;
> }
> @@ -702,7 +702,7 @@ 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_close (notmuch);
> + notmuch_database_destroy (notmuch);
> notmuch = NULL;
> }
>
> @@ -738,9 +738,19 @@ notmuch_database_close (notmuch_database_t *notmuch)
> }
>
> delete notmuch->term_gen;
> + notmuch->term_gen = NULL;
> delete notmuch->query_parser;
> + notmuch->query_parser = NULL;
> delete notmuch->xapian_db;
> + notmuch->xapian_db = NULL;
> delete notmuch->value_range_processor;
> + notmuch->value_range_processor = NULL;
> +}
> +
> +void
> +notmuch_database_destroy (notmuch_database_t *notmuch)
> +{
> + notmuch_database_close (notmuch);
> talloc_free (notmuch);
> }
>
> diff --git a/lib/notmuch.h b/lib/notmuch.h
> index 673c423..84c9265 100644
> --- a/lib/notmuch.h
> +++ b/lib/notmuch.h
> @@ -133,7 +133,7 @@ typedef struct _notmuch_filenames notmuch_filenames_t;
> *
> * After a successful call to notmuch_database_create, the returned
> * database will be open so the caller should call
> - * notmuch_database_close when finished with it.
> + * notmuch_database_destroy when finished with it.
> *
> * The database will not yet have any data in it
> * (notmuch_database_create itself is a very cheap function). Messages
> @@ -165,7 +165,7 @@ 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_close when finished with
> + * The caller should call notmuch_database_destroy when finished with
> * this database.
> *
> * In case of any failure, this function returns NULL, (after printing
> @@ -175,11 +175,18 @@ notmuch_database_t *
> notmuch_database_open (const char *path,
> notmuch_database_mode_t mode);
>
> -/* Close the given notmuch database, freeing all associated
> - * resources. See notmuch_database_open. */
> +/* Close the given notmuch database.
> + *
> + * This function is called by notmuch_database_destroy and can be
> + * called multiple times. */
This needs a comment explaining the implications of closing the
database. Perhaps something like this (modeled on
Xapian::Database::close's documentation)
/* Close the given notmuch database.
*
* After notmuch_database_close has been called, calls to other
* functions on objects derived from this database may either behave
* as if the database had not been closed (e.g., if the required data
* has been cached) or may fail with a
* NOTMUCH_STATUS_XAPIAN_EXCEPTION.
*
* notmuch_database_close can be called multiple times. Later calls
* have no affect.
*/
> void
> notmuch_database_close (notmuch_database_t *database);
>
> +/* Destroy the notmuch database freeing all associated
> + * resources */
This should mention that this also closes the database (currently you
mention this in the doc for notmuch_database_close, but it's a reverse
reference there; that could probably even be omitted). Perhaps
/* Destroy the notmuch database, closing it if necessary and freeing
* all associated resources. */
> +void
> +notmuch_database_destroy (notmuch_database_t *database);
> +
> /* Return the database path of the given database.
> *
> * The return value is a string owned by notmuch so should not be
More information about the notmuch
mailing list