On Thu, 02 Oct 2014, "W. Trevor King" <wking@tremily.us> wrote: > On Thu, Oct 02, 2014 at 03:19:08PM -0400, Austin Clements wrote: >> This patch simplifies notmuch_database_close to explicitly abort any >> outstanding transaction and then just call Database::close. This >> works for both read-only and read/write databases, takes care of >> committing changes, unifies the exception handling path, and codifies >> aborting outstanding transactions. > > I don't expect atomic blocks are particularly useful for read-only > connections. If they aren't, I'd quibble with the “This works for > both read-only…” wording above. If they are, I'd drop the read/write It's true that atomic sections aren't very useful on a read-only database, but we do allow them for symmetry. We also keep track of how deeply nested we are so notmuch_database_end_atomic can return a proper error message regardless of whether the database is read/write or read-only. But all I'm saying in the commit message is that Xapian::Database::close works for both read-only and read/write databases and will flush if necessary, so we don't have to worry about that. > check below: > >> + /* If there's an outstanding transaction, it's unclear if >> + * closing the Xapian database commits everything up to >> + * that transaction, or may discard committed (but >> + * unflushed) transactions. To be certain, explicitly >> + * cancel any outstanding transaction before closing. */ >> + if (notmuch->mode == NOTMUCH_DATABASE_MODE_READ_WRITE && >> + notmuch->atomic_nesting) >> + (static_cast <Xapian::WritableDatabase *> (notmuch->xapian_db)) >> + ->cancel_transaction (); The notmuch->mode check here is necessary because atomic_nesting can be non-zero on a read-only database (for the reason I mentioned above), but we had better not cast xapian_db to a WritableDatabase if it isn't a WritableDatabase. > Thats a very minor quibble though, and I'd be glad to see this patch > land as is. > > Cheers, > Trevor > > -- > This email may be signed or encrypted with GnuPG (http://www.gnupg.org). > For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy