Re: [PATCH 06/20] mime-node: track whole-message crypto state while walking the tree

Subject: Re: [PATCH 06/20] mime-node: track whole-message crypto state while walking the tree

Date: Fri, 15 Jun 2018 07:52:17 -0300

To: Daniel Kahn Gillmor, Notmuch Mail

Cc:

From: David Bremner


Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:

> Deliberately populate the message's cryptographic status while walking
> the MIME tree from the CLI.
> ---
>  mime-node.c | 27 ++++++++++++++++++++++++---
>  1 file changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/mime-node.c b/mime-node.c
> index cbff95d1..6ecd121d 100644
> --- a/mime-node.c
> +++ b/mime-node.c
> @@ -49,6 +49,9 @@ _mime_node_context_free (mime_node_context_t *res)
>      if (res->stream)
>  	g_object_unref (res->stream);
>  
> +    if (res->msg_crypto)
> +	_notmuch_message_crypto_cleanup (res->msg_crypto);
> +

I think I see how you got here via code movement, but I think I prefer
the destructor approach for "library" code in util/

>  	}
> +    } else {
> +	status = _notmuch_message_crypto_potential_payload (node->ctx->msg_crypto, part, parent ? parent->part : NULL, numchil

having _notmuch_message and _notmuch_message_crypto as prefixes is a bit
confusing; I momentarily expected this function to take a
notmuch_message_t * as it's first argument. I can live with the current
naming, but if you had any ideas for a non-colliding prefix, that might
save a few headaches in the long run.

>  static mime_node_t *
> -_mime_node_create (mime_node_t *parent, GMimeObject *part)
> +_mime_node_create (mime_node_t *parent, GMimeObject *part, int numchild)
>  {

I guess that the additional argument is just to pass through to the call

> +	status = _notmuch_message_crypto_potential_payload (node->ctx->msg_crypto, part, parent ? parent->part : NULL, numchild);

Maybe add a note to that effect in the commit message.
_______________________________________________
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Thread: