Re: [PATCH v2] lib: add return status to database close and destroy

Subject: Re: [PATCH v2] lib: add return status to database close and destroy

Date: Sun, 19 Jan 2014 19:28:28 -0400

To: Jani Nikula, notmuch@notmuchmail.org

Cc:

From: David Bremner


Jani Nikula <jani@nikula.org> writes:

> -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) {

as I mentioned on IRC, I don't really know what this piece of code is
useful for, but that's orthogonal to this patch.

> -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;
>  }

I guess we're implicitly claiming that talloc_free cannot possibly
return -1 in our use case? perhaps a cast to void and/or a comment would
be suitable.

>  const char *
> diff --git a/lib/notmuch.h b/lib/notmuch.h
> index 02604c5..7ac7118 100644
> --- a/lib/notmuch.h
> +++ b/lib/notmuch.h
> @@ -287,8 +287,16 @@ notmuch_database_open (const char *path,
>   *
>   * notmuch_database_close can be called multiple times.  Later calls
>   * have no effect.

Is it conceivable that the user might wish to retry closing if an
exception occurs? It feels like we ought to have an opinion here.

No functionality complaints...

d

Thread: