From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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).