On Thu 2017-10-19 08:52:46 -0300, David Bremner wrote:
> Daniel Kahn Gillmor <dkg@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