Re: [PATCH 5/7] util/repair: add _notmuch_repair_crypto_payload_skip_legacy_display

Subject: Re: [PATCH 5/7] util/repair: add _notmuch_repair_crypto_payload_skip_legacy_display

Date: Sat, 03 Aug 2019 12:15:30 -0300

To: Daniel Kahn Gillmor, Notmuch Mail

Cc:

From: David Bremner


Daniel Kahn Gillmor <dkg@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.

> +	    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?
_______________________________________________
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Thread: