On Fri, 15 Sep 2017, Daniel Kahn Gillmor <d...@fifthhorseman.net> wrote: > This prepares us for using the crypto object in both the library and > the client. > > i've prefixed notmuch_crypto with _ to indicate that while this can be > built into the library when needed, it's not something to be exported > or used externally.
You know, this would be considerably easier to review if this were split to separate patches: - prefixing notmuch_crypto_t and friends with _ - dropping notmuch_crypto_context_t in favour of using GMimeCryptoContext directly - moving the stuff to util - changing the notmuch_crypto_cleanup() return type I think the patch is fine, but I'd have much more confidence in each individual patch if this were split up than I have in everything together. BR, Jani. > --- > Makefile.local | 1 - > mime-node.c | 12 ++++++------ > notmuch-client.h | 27 +++------------------------ > notmuch-reply.c | 2 +- > notmuch-show.c | 2 +- > util/Makefile.local | 2 +- > crypto.c => util/crypto.c | 45 +++++++++++++++++++++++++-------------------- > util/crypto.h | 31 +++++++++++++++++++++++++++++++ > 8 files changed, 68 insertions(+), 54 deletions(-) > rename crypto.c => util/crypto.c (79%) > create mode 100644 util/crypto.h > > diff --git a/Makefile.local b/Makefile.local > index 9d9c52c2..9505b7fe 100644 > --- a/Makefile.local > +++ b/Makefile.local > @@ -246,7 +246,6 @@ notmuch_client_srcs = \ > sprinter-text.c \ > query-string.c \ > mime-node.c \ > - crypto.c \ > tag-util.c > > notmuch_client_modules = $(notmuch_client_srcs:.c=.o) > diff --git a/mime-node.c b/mime-node.c > index 24d73afa..d9ff7de1 100644 > --- a/mime-node.c > +++ b/mime-node.c > @@ -33,7 +33,7 @@ typedef struct mime_node_context { > GMimeMessage *mime_message; > > /* Context provided by the caller. */ > - notmuch_crypto_t *crypto; > + _notmuch_crypto_t *crypto; > } mime_node_context_t; > > static int > @@ -56,7 +56,7 @@ _mime_node_context_free (mime_node_context_t *res) > > notmuch_status_t > mime_node_open (const void *ctx, notmuch_message_t *message, > - notmuch_crypto_t *crypto, 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; > @@ -171,7 +171,7 @@ set_signature_list_destructor (mime_node_t *node) > /* Verify a signed mime node (GMime 2.6) */ > static void > node_verify (mime_node_t *node, GMimeObject *part, > - g_mime_3_unused(notmuch_crypto_context_t *cryptoctx)) > + g_mime_3_unused(GMimeCryptoContext *cryptoctx)) > { > GError *err = NULL; > > @@ -192,7 +192,7 @@ node_verify (mime_node_t *node, GMimeObject *part, > /* Decrypt and optionally verify an encrypted mime node (GMime 2.6) */ > static void > node_decrypt_and_verify (mime_node_t *node, GMimeObject *part, > - g_mime_3_unused(notmuch_crypto_context_t *cryptoctx)) > + g_mime_3_unused(GMimeCryptoContext *cryptoctx)) > { > GError *err = NULL; > GMimeDecryptResult *decrypt_result = NULL; > @@ -227,7 +227,7 @@ static mime_node_t * > _mime_node_create (mime_node_t *parent, GMimeObject *part) > { > mime_node_t *node = talloc_zero (parent, mime_node_t); > - notmuch_crypto_context_t *cryptoctx = NULL; > + GMimeCryptoContext *cryptoctx = NULL; > > /* Set basic node properties */ > node->part = part; > @@ -265,7 +265,7 @@ _mime_node_create (mime_node_t *parent, GMimeObject *part) > || (GMIME_IS_MULTIPART_SIGNED (part) && node->ctx->crypto->verify)) { > GMimeContentType *content_type = g_mime_object_get_content_type (part); > const char *protocol = g_mime_content_type_get_parameter (content_type, > "protocol"); > - cryptoctx = notmuch_crypto_get_context (node->ctx->crypto, protocol); > + cryptoctx = _notmuch_crypto_get_gmime_context (node->ctx->crypto, > protocol); > if (!cryptoctx) > return NULL; > } > diff --git a/notmuch-client.h b/notmuch-client.h > index 9d0f367d..76e69501 100644 > --- a/notmuch-client.h > +++ b/notmuch-client.h > @@ -31,10 +31,6 @@ > > #include "gmime-extra.h" > > -typedef GMimeCryptoContext notmuch_crypto_context_t; > -/* This is automatically included only since gmime 2.6.10 */ > -#include <gmime/gmime-pkcs7-context.h> > - > #include "notmuch.h" > > /* This is separate from notmuch-private.h because we're trying to > @@ -54,6 +50,7 @@ typedef GMimeCryptoContext notmuch_crypto_context_t; > #include <ctype.h> > > #include "talloc-extra.h" > +#include "crypto.h" > > #define unused(x) x __attribute__ ((unused)) > > @@ -71,22 +68,12 @@ typedef struct notmuch_show_format { > const struct notmuch_show_params *params); > } notmuch_show_format_t; > > -typedef struct notmuch_crypto { > - notmuch_bool_t verify; > - notmuch_bool_t decrypt; > -#if (GMIME_MAJOR_VERSION < 3) > - notmuch_crypto_context_t* gpgctx; > - notmuch_crypto_context_t* pkcs7ctx; > - const char *gpgpath; > -#endif > -} notmuch_crypto_t; > - > typedef struct notmuch_show_params { > notmuch_bool_t entire_thread; > notmuch_bool_t omit_excluded; > notmuch_bool_t output_body; > int part; > - notmuch_crypto_t crypto; > + _notmuch_crypto_t crypto; > notmuch_bool_t include_html; > GMimeStream *out_stream; > } notmuch_show_params_t; > @@ -180,14 +167,6 @@ typedef struct _notmuch_config notmuch_config_t; > void > notmuch_exit_if_unsupported_format (void); > > -#if (GMIME_MAJOR_VERSION <3) > -notmuch_crypto_context_t * > -notmuch_crypto_get_context (notmuch_crypto_t *crypto, const char *protocol); > -#endif > - > -int > -notmuch_crypto_cleanup (notmuch_crypto_t *crypto); > - > int > notmuch_count_command (notmuch_config_t *config, int argc, char *argv[]); > > @@ -448,7 +427,7 @@ struct mime_node { > */ > notmuch_status_t > mime_node_open (const void *ctx, notmuch_message_t *message, > - notmuch_crypto_t *crypto, 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 929f3077..00fff3ca 100644 > --- a/notmuch-reply.c > +++ b/notmuch-reply.c > @@ -759,7 +759,7 @@ notmuch_reply_command (notmuch_config_t *config, int > argc, char *argv[]) > if (do_reply (config, query, ¶ms, format, reply_all) != 0) > return EXIT_FAILURE; > > - notmuch_crypto_cleanup (¶ms.crypto); > + _notmuch_crypto_cleanup (¶ms.crypto); > notmuch_query_destroy (query); > notmuch_database_destroy (notmuch); > > diff --git a/notmuch-show.c b/notmuch-show.c > index cdcc2a98..a35b11ad 100644 > --- a/notmuch-show.c > +++ b/notmuch-show.c > @@ -1241,7 +1241,7 @@ notmuch_show_command (notmuch_config_t *config, int > argc, char *argv[]) > g_mime_stream_flush (params.out_stream); > g_object_unref (params.out_stream); > > - notmuch_crypto_cleanup (¶ms.crypto); > + _notmuch_crypto_cleanup (¶ms.crypto); > notmuch_query_destroy (query); > notmuch_database_destroy (notmuch); > > diff --git a/util/Makefile.local b/util/Makefile.local > index 3027880b..ba03230e 100644 > --- a/util/Makefile.local > +++ b/util/Makefile.local > @@ -5,7 +5,7 @@ extra_cflags += -I$(srcdir)/$(dir) > > libnotmuch_util_c_srcs := $(dir)/xutil.c $(dir)/error_util.c > $(dir)/hex-escape.c \ > $(dir)/string-util.c $(dir)/talloc-extra.c > $(dir)/zlib-extra.c \ > - $(dir)/util.c $(dir)/gmime-extra.c > + $(dir)/util.c $(dir)/gmime-extra.c $(dir)/crypto.c > > libnotmuch_util_modules := $(libnotmuch_util_c_srcs:.c=.o) > > diff --git a/crypto.c b/util/crypto.c > similarity index 79% > rename from crypto.c > rename to util/crypto.c > index cc45b885..97e8c8f4 100644 > --- a/crypto.c > +++ b/util/crypto.c > @@ -16,18 +16,26 @@ > * along with this program. If not, see https://www.gnu.org/licenses/ . > * > * Authors: Jameson Rollins <jroll...@finestructure.net> > + * Daniel Kahn Gillmor <d...@fifthhorseman.net> > */ > > -#include "notmuch-client.h" > +#include "notmuch.h" > +#include "crypto.h" > +#include "lib/notmuch-private.h" > +#include <string.h> > + > +#define ARRAY_SIZE(arr) (sizeof (arr) / sizeof (arr[0])) > + > #if (GMIME_MAJOR_VERSION < 3) > /* Create a GPG context (GMime 2.6) */ > -static notmuch_crypto_context_t * > -create_gpg_context (notmuch_crypto_t *crypto) > +static GMimeCryptoContext* > +create_gpg_context (_notmuch_crypto_t *crypto) > { > - notmuch_crypto_context_t *gpgctx; > + GMimeCryptoContext *gpgctx; > > - if (crypto->gpgctx) > + if (crypto->gpgctx) { > return crypto->gpgctx; > + } > > /* TODO: GMimePasswordRequestFunc */ > gpgctx = g_mime_gpg_context_new (NULL, crypto->gpgpath ? crypto->gpgpath > : "gpg"); > @@ -40,14 +48,14 @@ create_gpg_context (notmuch_crypto_t *crypto) > g_mime_gpg_context_set_use_agent ((GMimeGpgContext *) gpgctx, TRUE); > g_mime_gpg_context_set_always_trust ((GMimeGpgContext *) gpgctx, FALSE); > > - return gpgctx; > + return crypto->gpgctx; > } > > /* Create a PKCS7 context (GMime 2.6) */ > -static notmuch_crypto_context_t * > -create_pkcs7_context (notmuch_crypto_t *crypto) > +static GMimeCryptoContext* > +create_pkcs7_context (_notmuch_crypto_t *crypto) > { > - notmuch_crypto_context_t *pkcs7ctx; > + GMimeCryptoContext *pkcs7ctx; > > if (crypto->pkcs7ctx) > return crypto->pkcs7ctx; > @@ -63,11 +71,11 @@ create_pkcs7_context (notmuch_crypto_t *crypto) > g_mime_pkcs7_context_set_always_trust ((GMimePkcs7Context *) pkcs7ctx, > FALSE); > > - return pkcs7ctx; > + return crypto->pkcs7ctx; > } > static const struct { > const char *protocol; > - notmuch_crypto_context_t *(*get_context) (notmuch_crypto_t *crypto); > + GMimeCryptoContext *(*get_context) (_notmuch_crypto_t *crypto); > } protocols[] = { > { > .protocol = "application/pgp-signature", > @@ -89,10 +97,10 @@ static const struct { > > /* for the specified protocol return the context pointer (initializing > * if needed) */ > -notmuch_crypto_context_t * > -notmuch_crypto_get_context (notmuch_crypto_t *crypto, const char *protocol) > +GMimeCryptoContext * > +_notmuch_crypto_get_gmime_context (_notmuch_crypto_t *crypto, const char > *protocol) > { > - notmuch_crypto_context_t *cryptoctx = NULL; > + GMimeCryptoContext *cryptoctx = NULL; > size_t i; > > if (! protocol) { > @@ -117,8 +125,8 @@ notmuch_crypto_get_context (notmuch_crypto_t *crypto, > const char *protocol) > return NULL; > } > > -int > -notmuch_crypto_cleanup (notmuch_crypto_t *crypto) > +void > +_notmuch_crypto_cleanup (_notmuch_crypto_t *crypto) > { > if (crypto->gpgctx) { > g_object_unref (crypto->gpgctx); > @@ -129,12 +137,9 @@ notmuch_crypto_cleanup (notmuch_crypto_t *crypto) > g_object_unref (crypto->pkcs7ctx); > crypto->pkcs7ctx = NULL; > } > - > - return 0; > } > #else > -int notmuch_crypto_cleanup (unused(notmuch_crypto_t *crypto)) > +void _notmuch_crypto_cleanup (unused(_notmuch_crypto_t *crypto)) > { > - return 0; > } > #endif > diff --git a/util/crypto.h b/util/crypto.h > new file mode 100644 > index 00000000..6d15a6ae > --- /dev/null > +++ b/util/crypto.h > @@ -0,0 +1,31 @@ > +#ifndef _CRYPTO_H > +#define _CRYPTO_H > + > +#include "notmuch.h" > +#if (GMIME_MAJOR_VERSION < 3) > +#include "gmime-extra.h" > +#include <gmime/gmime.h> > +/* This is automatically included only since gmime 2.6.10 */ > +#include <gmime/gmime-pkcs7-context.h> > +#endif > + > +typedef struct _notmuch_crypto { > + notmuch_bool_t verify; > + notmuch_bool_t decrypt; > +#if (GMIME_MAJOR_VERSION < 3) > + GMimeCryptoContext* gpgctx; > + GMimeCryptoContext* pkcs7ctx; > + const char *gpgpath; > +#endif > +} _notmuch_crypto_t; > + > + > +#if (GMIME_MAJOR_VERSION < 3) > +GMimeCryptoContext * > +_notmuch_crypto_get_gmime_context (_notmuch_crypto_t *crypto, const char > *protocol); > +#endif > + > +void > +_notmuch_crypto_cleanup (_notmuch_crypto_t *crypto); > + > +#endif > -- > 2.14.1 > > _______________________________________________ > notmuch mailing list > notmuch@notmuchmail.org > https://notmuchmail.org/mailman/listinfo/notmuch _______________________________________________ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch