On Thu, 17 May 2012, Jameson Graef Rollins <jrollins at finestructure.net>
wrote:
> The main point here is to keep track of the crypto stuff together in
> one place. In notmuch-show the crypto struct is a sub structure of
> the parameters struct. In notmuch-reply, which had been using a
> notmuch_show_params_t to store the crypto parameters, we can now just
> use the general crypto struct.
Looks good. My only (potentially unwarranted) worry is dropping the use
of notmuch_show_params_t in reply. This diverges the show/reply code,
making any future unification of them slightly harder. And if reply ever
needs params, we'll need to bring it back. But perhaps I worry too
much. :)
BR,
Jani.
>
> I slip in a name change of the crypto context itself to better reflect
> what the context is specifically for: it's actually a GPG context,
> which is a sub type of Crypto context. There are other types of
> Crypto contexts (Pkcs7 in particular) so we want to be clear.
>
> The following patches will use this to simplify some function
> interfaces.
> ---
> notmuch-client.h | 16 ++++++++++------
> notmuch-reply.c | 34 +++++++++++++++++-----------------
> notmuch-show.c | 22 +++++++++++-----------
> 3 files changed, 38 insertions(+), 34 deletions(-)
>
> diff --git a/notmuch-client.h b/notmuch-client.h
> index 19b7f01..2ad24cf 100644
> --- a/notmuch-client.h
> +++ b/notmuch-client.h
> @@ -74,17 +74,21 @@ typedef struct notmuch_show_format {
> const char *message_set_end;
> } notmuch_show_format_t;
>
> +typedef struct notmuch_crypto {
> +#ifdef GMIME_ATLEAST_26
> + GMimeCryptoContext* gpgctx;
> +#else
> + GMimeCipherContext* gpgctx;
> +#endif
> + notmuch_bool_t decrypt;
> +} notmuch_crypto_t;
> +
> typedef struct notmuch_show_params {
> notmuch_bool_t entire_thread;
> notmuch_bool_t omit_excluded;
> notmuch_bool_t raw;
> int part;
> -#ifdef GMIME_ATLEAST_26
> - GMimeCryptoContext* cryptoctx;
> -#else
> - GMimeCipherContext* cryptoctx;
> -#endif
> - notmuch_bool_t decrypt;
> + notmuch_crypto_t crypto;
> } notmuch_show_params_t;
>
> /* There's no point in continuing when we've detected that we've done
> diff --git a/notmuch-reply.c b/notmuch-reply.c
> index 7184a5d..ed87899 100644
> --- a/notmuch-reply.c
> +++ b/notmuch-reply.c
> @@ -515,7 +515,7 @@ static int
> notmuch_reply_format_default(void *ctx,
> notmuch_config_t *config,
> notmuch_query_t *query,
> - notmuch_show_params_t *params,
> + notmuch_crypto_t *crypto,
> notmuch_bool_t reply_all)
> {
> GMimeMessage *reply;
> @@ -544,7 +544,7 @@ notmuch_reply_format_default(void *ctx,
> g_object_unref (G_OBJECT (reply));
> reply = NULL;
>
> - if (mime_node_open (ctx, message, params->cryptoctx, params->decrypt,
> + if (mime_node_open (ctx, message, crypto->gpgctx, crypto->decrypt,
> &root) == NOTMUCH_STATUS_SUCCESS) {
> format_part_reply (root);
> talloc_free (root);
> @@ -559,7 +559,7 @@ static int
> notmuch_reply_format_json(void *ctx,
> notmuch_config_t *config,
> notmuch_query_t *query,
> - notmuch_show_params_t *params,
> + notmuch_crypto_t *crypto,
> notmuch_bool_t reply_all)
> {
> GMimeMessage *reply;
> @@ -574,7 +574,7 @@ notmuch_reply_format_json(void *ctx,
>
> messages = notmuch_query_search_messages (query);
> message = notmuch_messages_get (messages);
> - if (mime_node_open (ctx, message, params->cryptoctx, params->decrypt,
> + if (mime_node_open (ctx, message, crypto->gpgctx, crypto->decrypt,
> &node) != NOTMUCH_STATUS_SUCCESS)
> return 1;
>
> @@ -605,7 +605,7 @@ static int
> notmuch_reply_format_headers_only(void *ctx,
> notmuch_config_t *config,
> notmuch_query_t *query,
> - unused (notmuch_show_params_t *params),
> + unused (notmuch_crypto_t *crypto),
> notmuch_bool_t reply_all)
> {
> GMimeMessage *reply;
> @@ -674,8 +674,8 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
> notmuch_query_t *query;
> char *query_string;
> int opt_index, ret = 0;
> - int (*reply_format_func)(void *ctx, notmuch_config_t *config,
> notmuch_query_t *query, notmuch_show_params_t *params, notmuch_bool_t
> reply_all);
> - notmuch_show_params_t params = { .part = -1 };
> + int (*reply_format_func)(void *ctx, notmuch_config_t *config,
> notmuch_query_t *query, notmuch_crypto_t *crypto, notmuch_bool_t reply_all);
> + notmuch_crypto_t crypto = { .decrypt = FALSE };
> int format = FORMAT_DEFAULT;
> int reply_all = TRUE;
>
> @@ -689,7 +689,7 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
> (notmuch_keyword_t []){ { "all", TRUE },
> { "sender", FALSE },
> { 0, 0 } } },
> - { NOTMUCH_OPT_BOOLEAN, ¶ms.decrypt, "decrypt", 'd', 0 },
> + { NOTMUCH_OPT_BOOLEAN, &crypto.decrypt, "decrypt", 'd', 0 },
> { 0, 0, 0, 0, 0 }
> };
>
> @@ -706,18 +706,18 @@ notmuch_reply_command (void *ctx, int argc, char
> *argv[])
> else
> reply_format_func = notmuch_reply_format_default;
>
> - if (params.decrypt) {
> + if (crypto.decrypt) {
> #ifdef GMIME_ATLEAST_26
> /* TODO: GMimePasswordRequestFunc */
> - params.cryptoctx = g_mime_gpg_context_new (NULL, "gpg");
> + crypto.gpgctx = g_mime_gpg_context_new (NULL, "gpg");
> #else
> GMimeSession* session = g_object_new (g_mime_session_get_type(), NULL);
> - params.cryptoctx = g_mime_gpg_context_new (session, "gpg");
> + crypto.gpgctx = g_mime_gpg_context_new (session, "gpg");
> #endif
> - if (params.cryptoctx) {
> - g_mime_gpg_context_set_always_trust ((GMimeGpgContext*)
> params.cryptoctx, FALSE);
> + if (crypto.gpgctx) {
> + g_mime_gpg_context_set_always_trust ((GMimeGpgContext*)
> crypto.gpgctx, FALSE);
> } else {
> - params.decrypt = FALSE;
> + crypto.decrypt = FALSE;
> fprintf (stderr, "Failed to construct gpg context.\n");
> }
> #ifndef GMIME_ATLEAST_26
> @@ -750,14 +750,14 @@ notmuch_reply_command (void *ctx, int argc, char
> *argv[])
> return 1;
> }
>
> - if (reply_format_func (ctx, config, query, ¶ms, reply_all) != 0)
> + if (reply_format_func (ctx, config, query, &crypto, reply_all) != 0)
> return 1;
>
> notmuch_query_destroy (query);
> notmuch_database_destroy (notmuch);
>
> - if (params.cryptoctx)
> - g_object_unref(params.cryptoctx);
> + if (crypto.gpgctx)
> + g_object_unref(crypto.gpgctx);
>
> return ret;
> }
> diff --git a/notmuch-show.c b/notmuch-show.c
> index 95427d4..d254179 100644
> --- a/notmuch-show.c
> +++ b/notmuch-show.c
> @@ -810,8 +810,8 @@ show_message (void *ctx,
> mime_node_t *root, *part;
> notmuch_status_t status;
>
> - status = mime_node_open (local, message, params->cryptoctx,
> - params->decrypt, &root);
> + status = mime_node_open (local, message, params->crypto.gpgctx,
> + params->crypto.decrypt, &root);
> if (status)
> goto DONE;
> part = mime_node_seek_dfs (root, (params->part < 0 ? 0 : params->part));
> @@ -1002,7 +1002,7 @@ notmuch_show_command (void *ctx, unused (int argc),
> unused (char *argv[]))
> { 0, 0 } } },
> { NOTMUCH_OPT_INT, ¶ms.part, "part", 'p', 0 },
> { NOTMUCH_OPT_BOOLEAN, ¶ms.entire_thread, "entire-thread", 't', 0 },
> - { NOTMUCH_OPT_BOOLEAN, ¶ms.decrypt, "decrypt", 'd', 0 },
> + { NOTMUCH_OPT_BOOLEAN, ¶ms.crypto.decrypt, "decrypt", 'd', 0 },
> { NOTMUCH_OPT_BOOLEAN, &verify, "verify", 'v', 0 },
> { 0, 0, 0, 0, 0 }
> };
> @@ -1047,18 +1047,18 @@ notmuch_show_command (void *ctx, unused (int argc),
> unused (char *argv[]))
> break;
> }
>
> - if (params.decrypt || verify) {
> + if (params.crypto.decrypt || verify) {
> #ifdef GMIME_ATLEAST_26
> /* TODO: GMimePasswordRequestFunc */
> - params.cryptoctx = g_mime_gpg_context_new (NULL, "gpg");
> + params.crypto.gpgctx = g_mime_gpg_context_new (NULL, "gpg");
> #else
> GMimeSession* session = g_object_new (g_mime_session_get_type(), NULL);
> - params.cryptoctx = g_mime_gpg_context_new (session, "gpg");
> + params.crypto.gpgctx = g_mime_gpg_context_new (session, "gpg");
> #endif
> - if (params.cryptoctx) {
> - g_mime_gpg_context_set_always_trust ((GMimeGpgContext*)
> params.cryptoctx, FALSE);
> + if (params.crypto.gpgctx) {
> + g_mime_gpg_context_set_always_trust ((GMimeGpgContext*)
> params.crypto.gpgctx, FALSE);
> } else {
> - params.decrypt = FALSE;
> + params.crypto.decrypt = FALSE;
> fprintf (stderr, "Failed to construct gpg context.\n");
> }
> #ifndef GMIME_ATLEAST_26
> @@ -1118,8 +1118,8 @@ notmuch_show_command (void *ctx, unused (int argc),
> unused (char *argv[]))
> notmuch_query_destroy (query);
> notmuch_database_destroy (notmuch);
>
> - if (params.cryptoctx)
> - g_object_unref(params.cryptoctx);
> + if (params.crypto.gpgctx)
> + g_object_unref(params.crypto.gpgctx);
>
> return ret;
> }
> --
> 1.7.10
>
> _______________________________________________
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
