On Mon 2019-05-27 07:24:41 -0300, David Bremner wrote: > Daniel Kahn Gillmor <d...@fifthhorseman.net> writes: > >> + status = _notmuch_message_crypto_potential_payload (msg_crypto, clear, >> GMIME_OBJECT (encrypted_data), GMIME_MULTIPART_ENCRYPTED_CONTENT); >> + _index_mime_part (message, indexopts, clear, msg_crypto); >> g_object_unref (clear); > > If you're going to ignore the return value here (not sure if that's a > good idea?) please explicitly cast to void rather than putting in > status to ignore.
Good catch, thanks. I've logged the error with _notmuch_database_log_append in v3 of this patch, i hope that makes sense to you! i note that _index_encrypted_mime_part() itself uses an odd mixture of _notmuch_database_log_append and _notmuch_database_log, which maybe is a sign that more cleanup is due there. I confess i always get a bit confused about when to use one vs. the other, though. _log resets the database's status_string, while _log_append just appends to it. On IRC, you wrote "I'd say it makes sense to append for warnings", which is a plausible rule of thumb, but seems like it might not map to the intent of how we expect the status_string to be used -- is it for the caller of the library? or something else? For example, should every call into the library reset the status string, and then there would only be one _database_log() function? (i don't know whether that's feasible, or what it would mean for internal code that already calls exported functions directly). It'd probably be worthwhile for someone to do an audit of those uses and come up with some normalized way of handling this that we can clearly explain, because i think it's a bit unwieldy now. I'm writing this report with the intent of tagging this e-mail with notmuch::bug, in the hopes that someone interested in doing maintenance work will take this on as a future project :) --dkg
Description: PGP signature
_______________________________________________ notmuch mailing list email@example.com https://notmuchmail.org/mailman/listinfo/notmuch