Quoth Jani Nikula on Dec 01 at 3:13 pm:
> notmuch_database_close may fail in Xapian ->flush() or ->close(), so
> report the status. Similarly for notmuch_database_destroy which calls
> close.
>
> This is required for notmuch insert to report error status if message
> indexing failed.
>
> Bump the notmuch version to allow clients to conditional build against
> both the current release and the next release (current git master).
> ---
> lib/database.cc | 18 ++++++++++++++----
> lib/notmuch.h | 17 ++++++++++++++---
> 2 files changed, 28 insertions(+), 7 deletions(-)
>
> diff --git a/lib/database.cc b/lib/database.cc
> index f395061..98e2c31 100644
> --- a/lib/database.cc
> +++ b/lib/database.cc
> @@ -767,14 +767,17 @@ notmuch_database_open (const char *path,
> return status;
> }
>
> -void
> +notmuch_status_t
> notmuch_database_close (notmuch_database_t *notmuch)
> {
> + notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
> +
> try {
> if (notmuch->xapian_db != NULL &&
> notmuch->mode == NOTMUCH_DATABASE_MODE_READ_WRITE)
> (static_cast <Xapian::WritableDatabase *>
> (notmuch->xapian_db))->flush ();
> } catch (const Xapian::Error &error) {
> + status = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
> if (! notmuch->exception_reported) {
> fprintf (stderr, "Error: A Xapian exception occurred flushing
> database: %s\n",
> error.get_msg().c_str());
> @@ -789,6 +792,7 @@ notmuch_database_close (notmuch_database_t *notmuch)
> notmuch->xapian_db->close();
> } catch (const Xapian::Error &error) {
> /* do nothing */
> + status = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
> }
> }
>
> @@ -802,6 +806,8 @@ notmuch_database_close (notmuch_database_t *notmuch)
> notmuch->value_range_processor = NULL;
> delete notmuch->date_range_processor;
> notmuch->date_range_processor = NULL;
> +
> + return status;
> }
>
> #if HAVE_XAPIAN_COMPACT
> @@ -966,7 +972,7 @@ notmuch_database_compact (const char *path,
>
> DONE:
> if (notmuch)
> - notmuch_database_destroy (notmuch);
> + ret = notmuch_database_destroy (notmuch);
This will clobber the error code on most of the error paths. Maybe
you want to only set ret if it's currently NOTMUCH_STATUS_SUCCESS?
>
> talloc_free (local);
>
> @@ -984,11 +990,15 @@ notmuch_database_compact (unused (const char *path),
> }
> #endif
>
> -void
> +notmuch_status_t
> notmuch_database_destroy (notmuch_database_t *notmuch)
> {
> - notmuch_database_close (notmuch);
> + notmuch_status_t status;
> +
> + status = notmuch_database_close (notmuch);
> talloc_free (notmuch);
> +
> + return status;
> }
>
> const char *
> diff --git a/lib/notmuch.h b/lib/notmuch.h
> index 7c3a30c..dbdce86 100644
> --- a/lib/notmuch.h
> +++ b/lib/notmuch.h
> @@ -42,7 +42,7 @@ NOTMUCH_BEGIN_DECLS
> #endif
>
> #define NOTMUCH_MAJOR_VERSION 0
> -#define NOTMUCH_MINOR_VERSION 17
> +#define NOTMUCH_MINOR_VERSION 18
> #define NOTMUCH_MICRO_VERSION 0
This is actually what got me thinking about
id:[email protected] (which obviously
conflicts). In particular, the Python bindings don't have access to
these macros, and can only use the soname version. (I think the Go
ones can use the macros; the Ruby ones certainly can.)
>
> /*
> @@ -236,8 +236,16 @@ notmuch_database_open (const char *path,
> *
> * notmuch_database_close can be called multiple times. Later calls
> * have no effect.
> + *
> + * Return value:
> + *
> + * NOTMUCH_STATUS_SUCCESS: Successfully closed the database.
> + *
> + * NOTMUCH_STATUS_XAPIAN_EXCEPTION: A Xapian exception occurred; the
> + * database has been closed but there are no guarantees the
> + * changes to the database, if any, have been flushed to disk.
> */
> -void
> +notmuch_status_t
> notmuch_database_close (notmuch_database_t *database);
>
> /* A callback invoked by notmuch_database_compact to notify the user
> @@ -263,8 +271,11 @@ notmuch_database_compact (const char* path,
>
> /* Destroy the notmuch database, closing it if necessary and freeing
> * all associated resources.
> + *
> + * Return value as in notmuch_database_close if the database was open;
> + * notmuch_database_destroy itself has no failure modes.
> */
> -void
> +notmuch_status_t
> notmuch_database_destroy (notmuch_database_t *database);
>
> /* Return the database path of the given database.
Regarding bindings (since you asked me to think about them), these
should be a safe changes from an ABI perspective (though obviously the
bindings will need changes to take advantage of the new return code).
_______________________________________________
notmuch mailing list
[email protected]
http://notmuchmail.org/mailman/listinfo/notmuch
