Date: Thu, 17 May 2012 18:26:47 -0400
From: Austin Clements <amdragon@MIT.EDU>
To: Jameson Graef Rollins <jrollins@finestructure.net>
Subject: Re: [PATCH 3/6] cli: modify mime_node_open to take crypto struct as
	argument
Message-ID: <20120517222647.GP11804@mit.edu>
References: <1337205359-2444-1-git-send-email-jrollins@finestructure.net>
	<1337205359-2444-2-git-send-email-jrollins@finestructure.net>
	<1337205359-2444-3-git-send-email-jrollins@finestructure.net>
	<1337205359-2444-4-git-send-email-jrollins@finestructure.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1337205359-2444-4-git-send-email-jrollins@finestructure.net>
User-Agent: Mutt/1.5.21 (2010-09-15)
Cc: Notmuch Mail <notmuch@notmuchmail.org>
Precedence: list

Quoth Jameson Graef Rollins on May 16 at  2:55 pm:
> Again, for interface simplification and getting rid of more #ifdefs.
> ---
>  mime-node.c      |   10 ++--------
>  notmuch-client.h |   14 +++++---------
>  notmuch-reply.c  |    6 ++----
>  notmuch-show.c   |    3 +--
>  4 files changed, 10 insertions(+), 23 deletions(-)
> 
> diff --git a/mime-node.c b/mime-node.c
> index 79a3654..4faeffc 100644
> --- a/mime-node.c
> +++ b/mime-node.c
> @@ -56,12 +56,7 @@ _mime_node_context_free (mime_node_context_t *res)
>  
>  notmuch_status_t
>  mime_node_open (const void *ctx, notmuch_message_t *message,
> -#ifdef GMIME_ATLEAST_26
> -		GMimeCryptoContext *cryptoctx,
> -#else
> -		GMimeCipherContext *cryptoctx,
> -#endif
> -		notmuch_bool_t decrypt, mime_node_t **root_out)
> +		notmuch_crypto_t *crypto, mime_node_t **root_out)
>  {
>      const char *filename = notmuch_message_get_filename (message);
>      mime_node_context_t *mctx;
> @@ -113,8 +108,7 @@ mime_node_open (const void *ctx, notmuch_message_t *message,
>  	goto DONE;
>      }
>  
> -    mctx->crypto.gpgctx = cryptoctx;
> -    mctx->crypto.decrypt = decrypt;
> +    mctx->crypto = *crypto;

I think you want to store the notmuch_crypto_t* pointer in the
mime_node_t, rather than copying the value.  This doesn't matter so
much at this point, but when you later start lazily constructing the
cyrpto context, storing it by value will force you to lazily
initialize it separately potentially for every mime_node_t instance.

>  
>      /* Create the root node */
>      root->part = GMIME_OBJECT (mctx->mime_message);
> diff --git a/notmuch-client.h b/notmuch-client.h
> index 2ad24cf..d86fab3 100644
> --- a/notmuch-client.h
> +++ b/notmuch-client.h
> @@ -345,9 +345,10 @@ struct mime_node {
>  };
>  
>  /* Construct a new MIME node pointing to the root message part of
> - * message.  If cryptoctx is non-NULL, it will be used to verify
> - * signatures on any child parts.  If decrypt is true, then cryptoctx
> - * will additionally be used to decrypt any encrypted child parts.
> + * message.  If crypto.gpgctx is non-NULL, it will be used to verify
> + * signatures on any child parts.  If crypto.decrypt is true, then
> + * crypto.gpgctx will additionally be used to decrypt any encrypted
> + * child parts.
>   *
>   * Return value:
>   *
> @@ -359,12 +360,7 @@ struct mime_node {
>   */
>  notmuch_status_t
>  mime_node_open (const void *ctx, notmuch_message_t *message,
> -#ifdef GMIME_ATLEAST_26
> -		GMimeCryptoContext *cryptoctx,
> -#else
> -		GMimeCipherContext *cryptoctx,
> -#endif
> -		notmuch_bool_t decrypt, mime_node_t **node_out);
> +		notmuch_crypto_t *crypto, mime_node_t **node_out);
>  
>  /* Return a new MIME node for the requested child part of parent.
>   * parent will be used as the talloc context for the returned child
> diff --git a/notmuch-reply.c b/notmuch-reply.c
> index ed87899..6662adb 100644
> --- a/notmuch-reply.c
> +++ b/notmuch-reply.c
> @@ -544,8 +544,7 @@ notmuch_reply_format_default(void *ctx,
>  	g_object_unref (G_OBJECT (reply));
>  	reply = NULL;
>  
> -	if (mime_node_open (ctx, message, crypto->gpgctx, crypto->decrypt,
> -			    &root) == NOTMUCH_STATUS_SUCCESS) {
> +	if (mime_node_open (ctx, message, crypto, &root) == NOTMUCH_STATUS_SUCCESS) {
>  	    format_part_reply (root);
>  	    talloc_free (root);
>  	}
> @@ -574,8 +573,7 @@ notmuch_reply_format_json(void *ctx,
>  
>      messages = notmuch_query_search_messages (query);
>      message = notmuch_messages_get (messages);
> -    if (mime_node_open (ctx, message, crypto->gpgctx, crypto->decrypt,
> -			&node) != NOTMUCH_STATUS_SUCCESS)
> +    if (mime_node_open (ctx, message, crypto, &node) != NOTMUCH_STATUS_SUCCESS)
>  	return 1;
>  
>      reply = create_reply_message (ctx, config, message, reply_all);
> diff --git a/notmuch-show.c b/notmuch-show.c
> index d254179..8b4d308 100644
> --- a/notmuch-show.c
> +++ b/notmuch-show.c
> @@ -810,8 +810,7 @@ show_message (void *ctx,
>      mime_node_t *root, *part;
>      notmuch_status_t status;
>  
> -    status = mime_node_open (local, message, params->crypto.gpgctx,
> -			     params->crypto.decrypt, &root);
> +    status = mime_node_open (local, message, &(params->crypto), &root);
>      if (status)
>  	goto DONE;
>      part = mime_node_seek_dfs (root, (params->part < 0 ? 0 : params->part));