Re: [PATCH v2 5/7] lib: return NOTMUCH_STATUS_OPERATION_INVALIDATED where appropriate

Subject: Re: [PATCH v2 5/7] lib: return NOTMUCH_STATUS_OPERATION_INVALIDATED where appropriate

Date: Mon, 4 Aug 2025 20:39:07 +0100

To: David Bremner

Cc: notmuch@notmuchmail.org, xapian-discuss@lists.xapian.org

From: Olly Betts


On Mon, Aug 04, 2025 at 11:11:31AM -0300, David Bremner wrote:
> > +static inline notmuch_private_status_t
> > +_notmuch_xapian_errorp (const Xapian::Error &error)
> > +{
> > +    const char *type = error.get_type();
> > +	return (! strcmp (type, "DatabaseModifiedError")) ?
> > +	    NOTMUCH_PRIVATE_STATUS_OPERATION_INVALIDATED : NOTMUCH_PRIVATE_STATUS_XAPIAN_EXCEPTION;
> > +}
> 
> My first reaction to using strcmp here is ugh, but I guess that's the
> intended method of introspection. I have CC'ed xapian-users, in case
> someone there wants to comment. Naively, it seems like a case where
> typeid would be useful, but I am not enough of a C++ expert to
> understand why Xapian doesn't do that.

In C++ the natural approach is to use multiple catch clauses to handle
some error subclasses differently, i.e. something like:

    try {
        do_something_with_xapian();
    } catch (const Xapian::DatabaseModifiedError&) {
        do_something_special();
    } catch (const Xapian::Error& e) {
        report_error(e);
    }

get_type() is more intended to provide a way to report the type for
logging exceptions, etc.  It should work as above though, and it's
an exception case so performance shouldn't really matter (but I'd
guess it actually would be fairly efficient as string comparisons
should be well optimised).

Cheers,
    Olly
_______________________________________________
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-leave@notmuchmail.org

Thread: