Hi Bremner-- thanks for the review!
On Sat 2019-08-03 12:15:30 -0300, David Bremner wrote:
> Daniel Kahn Gillmor <d...@fifthhorseman.net> writes:
>
>> + ret = true;
>> + for (int i = 0; i < g_mime_header_list_get_count
>> (legacy_display_headers); i++) {
>> + GMimeHeader *dh = g_mime_header_list_get_header_at
>> (legacy_display_headers, i);
>> + if (dh == NULL) {
>> + ret = false;
>> + break;
>> + }
>
> I can live with the use of break if you think it's superior, but I think
> the idiom of "goto DONE" is more common in the notmuch codebase. I
> personally always have think about the semantics of "break" and
> "continue" in C pretty carefully.
i thought i was the only one who got confused between "break" and
"continue"! I will convert to goto DONE, i agree it's more readable.
>> + if (strcmp (g_mime_header_get_value (dh), g_mime_header_get_value
>> (ph))) {
>> + ret = false;
>> + break;
>> + }
>
> It's not really clear to me what kind of "invalid" causes
> g_mime_header_get_value to return NULL. Maybe this strcmp should be
> guarded against that?
i think it's impossible in the current implementation for this to go
wrong, since we've already got a GMimeHeader object from an existing
block of headers, but i'll add some protection just in case GMime
changes its implementation or some fuzzer constructs a truly devious
not-quite-RFC-822 input.
--dkg
signature.asc
Description: PGP signature
_______________________________________________ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
