[PATCH v6 06/14] reindex: drop all properties named with prefix "index."

Daniel Kahn Gillmor dkg at fifthhorseman.net
Thu Oct 19 22:24:50 PDT 2017


On Thu 2017-10-19 08:52:46 -0300, David Bremner wrote:
> Daniel Kahn Gillmor <dkg at fifthhorseman.net> writes:
>
>> +    ret = notmuch_message_remove_all_properties_with_prefix (message, "index.");
>> +    if (ret) {
>> +	INTERNAL_ERROR ("failed to remove index.* properties");
>> +	goto DONE;
>> +    }
>
> 1) INTERNAL_ERROR is fatal, so the goto is unneeded / confusing.
>
> 2) Is there no sensible return value here? The function already has an
> error return path. I see there is one INTERNAL_ERROR there so maybe
> similar reasoning applies and a TODO is enough.

I suppose the error returned by
notmuch_message_remove_all_properties_with_prefix() is the value that we
should return.  I've switched this to a simple goto DONE in my gitlab
repo.

However, i note that an error here will be (from the outside)
indistinguishable from an error in _notmuch_database_ensure_writable or
_notmuch_message_remove_indexed_terms() immediately above, so i've added
a small TODO anyway.

thanks for the reviews, bremner.  please keep them coming!  (or tell me
when you're done so i can send v7 to the list with resolutions to all
these nitpicks)

  --dkg
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20171020/63685b10/attachment.sig>


More information about the notmuch mailing list