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 A9E5F431FC0 for ; Sun, 19 Jan 2014 15:28:39 -0800 (PST) X-Virus-Scanned: Debian amavisd-new at olra.theworths.org X-Spam-Flag: NO X-Spam-Score: 0 X-Spam-Level: X-Spam-Status: No, score=0 tagged_above=-999 required=5 tests=[none] 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 xpFKmAV41V5a for ; Sun, 19 Jan 2014 15:28:35 -0800 (PST) Received: from yantan.tethera.net (yantan.tethera.net [199.188.72.155]) (using TLSv1 with cipher DHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) by olra.theworths.org (Postfix) with ESMTPS id 95534431FBD for ; Sun, 19 Jan 2014 15:28:35 -0800 (PST) Received: from remotemail by yantan.tethera.net with local (Exim 4.80) (envelope-from ) id 1W51n6-0001pZ-Ez; Sun, 19 Jan 2014 19:28:32 -0400 Received: (nullmailer pid 8058 invoked by uid 1000); Sun, 19 Jan 2014 23:28:28 -0000 From: David Bremner To: Jani Nikula , notmuch@notmuchmail.org Subject: Re: [PATCH v2] lib: add return status to database close and destroy In-Reply-To: <1390152046-6509-1-git-send-email-jani@nikula.org> References: <1390152046-6509-1-git-send-email-jani@nikula.org> User-Agent: Notmuch/0.17+35~g3b36898 (http://notmuchmail.org) Emacs/24.3.1 (x86_64-pc-linux-gnu) Date: Sun, 19 Jan 2014 19:28:28 -0400 Message-ID: <87ob37k0v7.fsf@zancas.localnet> MIME-Version: 1.0 Content-Type: text/plain 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: Sun, 19 Jan 2014 23:28:39 -0000 Jani Nikula 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 (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