From: Austin Clements Date: Wed, 4 Dec 2013 16:31:32 +0000 (+1900) Subject: Re: [PATCH 1/2] lib: add return status to database close and destroy X-Git-Url: http://git.tremily.us/?a=commitdiff_plain;h=53c7a529959398d120484c1eea34d66aa13bac98;p=notmuch-archives.git Re: [PATCH 1/2] lib: add return status to database close and destroy --- diff --git a/97/aa7fbe7e1d130c6ac4a6d498f47047575ef99e b/97/aa7fbe7e1d130c6ac4a6d498f47047575ef99e new file mode 100644 index 000000000..4a12ad876 --- /dev/null +++ b/97/aa7fbe7e1d130c6ac4a6d498f47047575ef99e @@ -0,0 +1,223 @@ +Return-Path: +X-Original-To: notmuch@notmuchmail.org +Delivered-To: notmuch@notmuchmail.org +Received: from localhost (localhost [127.0.0.1]) + by olra.theworths.org (Postfix) with ESMTP id 34240431FC4 + for ; Wed, 4 Dec 2013 08:31:42 -0800 (PST) +X-Virus-Scanned: Debian amavisd-new at olra.theworths.org +X-Spam-Flag: NO +X-Spam-Score: -0.7 +X-Spam-Level: +X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5 + tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled +Received: from olra.theworths.org ([127.0.0.1]) + by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024) + with ESMTP id ainOKEtesqi2 for ; + Wed, 4 Dec 2013 08:31:36 -0800 (PST) +Received: from dmz-mailsec-scanner-4.mit.edu (dmz-mailsec-scanner-4.mit.edu + [18.9.25.15]) + (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) + (No client certificate requested) + by olra.theworths.org (Postfix) with ESMTPS id 55C7F431FC2 + for ; Wed, 4 Dec 2013 08:31:36 -0800 (PST) +X-AuditID: 1209190f-b7fb86d000000c36-c1-529f58e75452 +Received: from mailhub-auth-3.mit.edu ( [18.9.21.43]) + (using TLS with cipher AES256-SHA (256/256 bits)) + (Client did not present a certificate) + by dmz-mailsec-scanner-4.mit.edu (Symantec Messaging Gateway) with SMTP + id 7F.A6.03126.7E85F925; Wed, 4 Dec 2013 11:31:35 -0500 (EST) +Received: from outgoing.mit.edu (outgoing-auth-1.mit.edu [18.9.28.11]) + by mailhub-auth-3.mit.edu (8.13.8/8.9.2) with ESMTP id rB4GVYAN021678; + Wed, 4 Dec 2013 11:31:35 -0500 +Received: from awakening.csail.mit.edu (awakening.csail.mit.edu [18.26.4.91]) + (authenticated bits=0) + (User authenticated as amdragon@ATHENA.MIT.EDU) + by outgoing.mit.edu (8.13.8/8.12.4) with ESMTP id rB4GVW2d007285 + (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT); + Wed, 4 Dec 2013 11:31:34 -0500 +Received: from amthrax by awakening.csail.mit.edu with local (Exim 4.80) + (envelope-from ) + id 1VoFMK-0000cm-GD; Wed, 04 Dec 2013 11:31:32 -0500 +Date: Wed, 4 Dec 2013 11:31:32 -0500 +From: Austin Clements +To: Jani Nikula +Subject: Re: [PATCH 1/2] lib: add return status to database close and destroy +Message-ID: <20131204163132.GC8854@mit.edu> +References: + <29b808bb6bf051fe21b6a72f12bb4ad1badfbf97.1385903109.git.jani@nikula.org> +MIME-Version: 1.0 +Content-Type: text/plain; charset=us-ascii +Content-Disposition: inline +In-Reply-To: + <29b808bb6bf051fe21b6a72f12bb4ad1badfbf97.1385903109.git.jani@nikula.org> +User-Agent: Mutt/1.5.21 (2010-09-15) +X-Brightmail-Tracker: + H4sIAAAAAAAAA+NgFmpmleLIzCtJLcpLzFFi42IR4hTV1n0eMT/IYP49MYum6c4W12/OZHZg + 8rh1/zW7x7NVt5gDmKK4bFJSczLLUov07RK4MtYuWsFcsEi14tHTO+wNjDtkuxg5OSQETCSa + 7x1ngbDFJC7cW8/WxcjFISQwm0li4rd1jBDOBkaJlq8HoTKnmCR2Lb3JAuEsYZT4fnAvI0g/ + i4CKxKe/k5lBbDYBDYlt+5eDxUUEFCU2n9wPZjMLSEt8+93MBGILC/hK9L/tAKvnFdCW+Lzi + NjuILSRQJ3F5/iImiLigxMmZT1ggerUkbvx7CRTnAJuz/B8HSJhTIEzi5b1XYONFgU6YcnIb + 2wRGoVlIumch6Z6F0L2AkXkVo2xKbpVubmJmTnFqsm5xcmJeXmqRrolebmaJXmpK6SZGcFhL + 8u9g/HZQ6RCjAAejEg8vB/u8ICHWxLLiytxDjJIcTEqivB3h84OE+JLyUyozEosz4otKc1KL + DzFKcDArifBODgTK8aYkVlalFuXDpKQ5WJTEeW9y2AcJCaQnlqRmp6YWpBbBZGU4OJQkeMNB + hgoWpaanVqRl5pQgpJk4OEGG8wAN144AGV5ckJhbnJkOkT/FqCglDtEsAJLIKM2D64WlnVeM + 4kCvCPMeBKniAaYsuO5XQIOZgAY3P5gHMrgkESEl1cBoy5O64kHBArOSz7orp/Yoyq5YqP7k + Ca9q3p0kxqeXXOzFX+tJfDR4UnBhl9uaFS+n7590fcGRGa+u7Z+86IlSccqDE+dfl7sL6zo2 + REufcGdeN5HP7Nqd9eqVWxuYQ75V1b5a+tvKQuTiuqS8xsgOnrtNOku59gmv53zV2GN+xlxN + 6tPhByy/lViKMxINtZiLihMBNAsa9xYDAAA= +Cc: notmuch@notmuchmail.org +X-BeenThere: notmuch@notmuchmail.org +X-Mailman-Version: 2.1.13 +Precedence: list +List-Id: "Use and development of the notmuch mail system." + +List-Unsubscribe: , + +List-Archive: +List-Post: +List-Help: +List-Subscribe: , + +X-List-Received-Date: Wed, 04 Dec 2013 16:31:42 -0000 + +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 (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:1386173986-9624-1-git-send-email-amdragon@mit.edu (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).