On Thu 2017-10-19 08:52:46 -0300, David Bremner wrote: > Daniel Kahn Gillmor <[email protected]> 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
signature.asc
Description: PGP signature
_______________________________________________ notmuch mailing list [email protected] https://notmuchmail.org/mailman/listinfo/notmuch
