--- /dev/null
+Return-Path: <amdragon@mit.edu>\r
+X-Original-To: notmuch@notmuchmail.org\r
+Delivered-To: notmuch@notmuchmail.org\r
+Received: from localhost (localhost [127.0.0.1])\r
+ by olra.theworths.org (Postfix) with ESMTP id EC2B2431FB6\r
+ for <notmuch@notmuchmail.org>; Fri, 18 May 2012 12:22:01 -0700 (PDT)\r
+X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
+X-Spam-Flag: NO\r
+X-Spam-Score: -0.7\r
+X-Spam-Level: \r
+X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5\r
+ tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled\r
+Received: from olra.theworths.org ([127.0.0.1])\r
+ by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
+ with ESMTP id lUsFhapNj3dv for <notmuch@notmuchmail.org>;\r
+ Fri, 18 May 2012 12:22:01 -0700 (PDT)\r
+Received: from dmz-mailsec-scanner-6.mit.edu (DMZ-MAILSEC-SCANNER-6.MIT.EDU\r
+ [18.7.68.35])\r
+ by olra.theworths.org (Postfix) with ESMTP id EA93B431FAE\r
+ for <notmuch@notmuchmail.org>; Fri, 18 May 2012 12:22:00 -0700 (PDT)\r
+X-AuditID: 12074423-b7fcc6d0000008a8-3d-4fb6a158d41b\r
+Received: from mailhub-auth-2.mit.edu ( [18.7.62.36])\r
+ by dmz-mailsec-scanner-6.mit.edu (Symantec Messaging Gateway) with SMTP\r
+ id 93.48.02216.851A6BF4; Fri, 18 May 2012 15:22:00 -0400 (EDT)\r
+Received: from outgoing.mit.edu (OUTGOING-AUTH.MIT.EDU [18.7.22.103])\r
+ by mailhub-auth-2.mit.edu (8.13.8/8.9.2) with ESMTP id q4IJLxdg028457; \r
+ Fri, 18 May 2012 15:22:00 -0400\r
+Received: from awakening.csail.mit.edu (awakening.csail.mit.edu [18.26.4.91])\r
+ (authenticated bits=0)\r
+ (User authenticated as amdragon@ATHENA.MIT.EDU)\r
+ by outgoing.mit.edu (8.13.6/8.12.4) with ESMTP id q4IJLwB3022516\r
+ (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=NOT);\r
+ Fri, 18 May 2012 15:21:59 -0400 (EDT)\r
+Received: from amthrax by awakening.csail.mit.edu with local (Exim 4.77)\r
+ (envelope-from <amdragon@mit.edu>)\r
+ id 1SVSkQ-00073K-1p; Fri, 18 May 2012 15:21:58 -0400\r
+Date: Fri, 18 May 2012 15:21:57 -0400\r
+From: Austin Clements <amdragon@MIT.EDU>\r
+To: Jameson Graef Rollins <jrollins@finestructure.net>\r
+Subject: Re: [PATCH v2 5/5] cli: lazily create the crypto gpg context only\r
+ when needed\r
+Message-ID: <20120518192157.GV11804@mit.edu>\r
+References: <1337362357-31281-1-git-send-email-jrollins@finestructure.net>\r
+ <1337362357-31281-2-git-send-email-jrollins@finestructure.net>\r
+ <1337362357-31281-3-git-send-email-jrollins@finestructure.net>\r
+ <1337362357-31281-4-git-send-email-jrollins@finestructure.net>\r
+ <1337362357-31281-5-git-send-email-jrollins@finestructure.net>\r
+ <1337362357-31281-6-git-send-email-jrollins@finestructure.net>\r
+MIME-Version: 1.0\r
+Content-Type: text/plain; charset=us-ascii\r
+Content-Disposition: inline\r
+In-Reply-To: <1337362357-31281-6-git-send-email-jrollins@finestructure.net>\r
+User-Agent: Mutt/1.5.21 (2010-09-15)\r
+X-Brightmail-Tracker:\r
+ H4sIAAAAAAAAA+NgFmpileLIzCtJLcpLzFFi42IRYrdT0Y1YuM3f4O0DSYs9+7wsrt+cyezA\r
+ 5HH3NJfHs1W3mAOYorhsUlJzMstSi/TtErgyNqz5zVzwWq/i16zTbA2My1S7GDk5JARMJC5f\r
+ /scMYYtJXLi3nq2LkYtDSGAfo8S+yT0sEM4GRon9KzqgnJNMErPO/meGcJYwSnyc/QOsn0VA\r
+ VeLu1nVgNpuAhsS2/csZQWwRATOJni9/wGxmAS2JrRs/gNnCAhESbxsusYDYvAI6Elu73oDF\r
+ hQT6mSVW/tKHiAtKnJz5hAWm98a/l0xdjBxAtrTE8n8cIGFOAW+J2/MegJWICqhITDm5jW0C\r
+ o9AsJN2zkHTPQuhewMi8ilE2JbdKNzcxM6c4NVm3ODkxLy+1SNdMLzezRC81pXQTIzioXZR3\r
+ MP45qHSIUYCDUYmH9+Kkbf5CrIllxZW5hxglOZiURHl95wGF+JLyUyozEosz4otKc1KLDzFK\r
+ cDArifDOnA6U401JrKxKLcqHSUlzsCiJ82povfMTEkhPLEnNTk0tSC2CycpwcChJ8K5bANQo\r
+ WJSanlqRlplTgpBm4uAEGc4DNLwbpIa3uCAxtzgzHSJ/ilFRSpx3GUhCACSRUZoH1wtLOq8Y\r
+ xYFeEeadDVLFA0xYcN2vgAYzAQ2uZAMbXJKIkJJqYDT++vpUc4mCWRTbvIMLtL15n4fkP3Q1\r
+ tdsscMLdal+LbvferGYebu6djw+fk+n6tcXs6Sax5/d8JB8de7LmJPOrbay/vHX3h1Zxm7ja\r
+ JjuVBe8JXuvZL9Nme+4qQ0LS5LNfF5qGv7TXCAh++WFSQ21w3d/NhvsYmc8dFnLwNTuerCzf\r
+ 4WO6X4mlOCPRUIu5qDgRAMs+ntsVAwAA\r
+Cc: Notmuch Mail <notmuch@notmuchmail.org>\r
+X-BeenThere: notmuch@notmuchmail.org\r
+X-Mailman-Version: 2.1.13\r
+Precedence: list\r
+List-Id: "Use and development of the notmuch mail system."\r
+ <notmuch.notmuchmail.org>\r
+List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
+ <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
+List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
+List-Post: <mailto:notmuch@notmuchmail.org>\r
+List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
+List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
+ <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
+X-List-Received-Date: Fri, 18 May 2012 19:22:02 -0000\r
+\r
+Quoth Jameson Graef Rollins on May 18 at 10:32 am:\r
+> Move the creation of the crypto ctx into mime-node.c and create it\r
+> only when needed. This removes code duplication from notmuch-show and\r
+> notmuch-reply, and should speed up these functions considerably if the\r
+> crypto flags are provided but the messages don't have any\r
+> cryptographic parts.\r
+> ---\r
+> mime-node.c | 25 +++++++++++++++++++++++++\r
+> notmuch-client.h | 3 ++-\r
+> notmuch-reply.c | 19 -------------------\r
+> notmuch-show.c | 23 -----------------------\r
+> 4 files changed, 27 insertions(+), 43 deletions(-)\r
+> \r
+> diff --git a/mime-node.c b/mime-node.c\r
+> index 3adbe5a..592e0b6 100644\r
+> --- a/mime-node.c\r
+> +++ b/mime-node.c\r
+> @@ -182,6 +182,31 @@ _mime_node_create (mime_node_t *parent, GMimeObject *part)\r
+> return NULL;\r
+> }\r
+> \r
+> + /* Lazily create the gpgctx if it's needed and hasn't been initialized yet */\r
+> + if ((GMIME_IS_MULTIPART_ENCRYPTED (part) || GMIME_IS_MULTIPART_SIGNED (part))\r
+> + && (node->ctx->crypto->verify || node->ctx->crypto->decrypt)) {\r
+> + if (!node->ctx->crypto->gpgctx) {\r
+\r
+These if conditions could be combined, like\r
+\r
+ if ((GMIME_IS_MULTIPART_ENCRYPTED (part) || GMIME_IS_MULTIPART_SIGNED (part))\r
+ && (node->ctx->crypto->verify || node->ctx->crypto->decrypt)\r
+ && !node->ctx->crypto->gpgctx) {\r
+\r
+When I see two nested 'if's like this, I expect there to be an else\r
+part to the inner if or something after the inner if (why else would\r
+it be separate?) and then I wind up matching braces when I don't see\r
+anything. Also, one 'if' would save a level of indentation.\r
+\r
+> +#ifdef GMIME_ATLEAST_26\r
+> + /* TODO: GMimePasswordRequestFunc */\r
+> + node->ctx->crypto->gpgctx = g_mime_gpg_context_new (NULL, "gpg");\r
+> +#else\r
+> + GMimeSession* session = g_object_new (g_mime_session_get_type(), NULL);\r
+> + node->ctx->crypto->gpgctx = g_mime_gpg_context_new (session, "gpg");\r
+> + g_object_unref (session);\r
+> +#endif\r
+> + if (node->ctx->crypto->gpgctx) {\r
+> + g_mime_gpg_context_set_always_trust ((GMimeGpgContext*) node->ctx->crypto->gpgctx, FALSE);\r
+> + } else {\r
+> + /* If we fail to create the gpgctx set the verify and\r
+> + * decrypt flags to FALSE so we don't try to do any\r
+> + * further verification or decryption */\r
+> + node->ctx->crypto->verify = FALSE;\r
+> + node->ctx->crypto->decrypt = FALSE;\r
+> + fprintf (stderr, "Failed to construct gpg context.\n");\r
+> + }\r
+> + }\r
+> + }\r
+> +\r
+> /* Handle PGP/MIME parts */\r
+> if (GMIME_IS_MULTIPART_ENCRYPTED (part) && node->ctx->crypto->decrypt) {\r
+> if (node->nchildren != 2) {\r
+> diff --git a/notmuch-client.h b/notmuch-client.h\r
+> index c671c13..c79eaa9 100644\r
+> --- a/notmuch-client.h\r
+> +++ b/notmuch-client.h\r
+> @@ -348,7 +348,8 @@ struct mime_node {\r
+> /* Construct a new MIME node pointing to the root message part of\r
+> * message. If crypto->verify is true, signed child parts will be\r
+> * verified. If crypto->decrypt is true, encrypted child parts will be\r
+> - * decrypted.\r
+> + * decrypted. The GPG context crypto->gpgctx does not need to be\r
+> + * pre-initialized as it will be initialized lazily as needed.\r
+\r
+Perhaps "If crypto->gpgctx is NULL, it will be lazily initialized."?\r
+The variable does have to be "initialized", in the sense that it can't\r
+be uninitialized data.\r
+\r
+It's slightly awkward that it's the caller's responsibility to free\r
+this lazily constructed object. That should probably be documented.\r
+We could more carefully reference count it, but I think that would\r
+actually be worse because the reference count would probably bounce\r
+through zero frequently.\r
+\r
+> *\r
+> * Return value:\r
+> *\r
+> diff --git a/notmuch-reply.c b/notmuch-reply.c\r
+> index 345be76..1a61aa7 100644\r
+> --- a/notmuch-reply.c\r
+> +++ b/notmuch-reply.c\r
+> @@ -707,25 +707,6 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])\r
+> else\r
+> reply_format_func = notmuch_reply_format_default;\r
+> \r
+> - if (crypto.decrypt) {\r
+> -#ifdef GMIME_ATLEAST_26\r
+> - /* TODO: GMimePasswordRequestFunc */\r
+> - crypto.gpgctx = g_mime_gpg_context_new (NULL, "gpg");\r
+> -#else\r
+> - GMimeSession* session = g_object_new (g_mime_session_get_type(), NULL);\r
+> - crypto.gpgctx = g_mime_gpg_context_new (session, "gpg");\r
+> -#endif\r
+> - if (crypto.gpgctx) {\r
+> - g_mime_gpg_context_set_always_trust ((GMimeGpgContext*) crypto.gpgctx, FALSE);\r
+> - } else {\r
+> - crypto.decrypt = FALSE;\r
+> - fprintf (stderr, "Failed to construct gpg context.\n");\r
+> - }\r
+> -#ifndef GMIME_ATLEAST_26\r
+> - g_object_unref (session);\r
+> -#endif\r
+> - }\r
+> -\r
+> config = notmuch_config_open (ctx, NULL, NULL);\r
+> if (config == NULL)\r
+> return 1;\r
+> diff --git a/notmuch-show.c b/notmuch-show.c\r
+> index f4ee038..5f785d0 100644\r
+> --- a/notmuch-show.c\r
+> +++ b/notmuch-show.c\r
+> @@ -1056,29 +1056,6 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))\r
+> break;\r
+> }\r
+> \r
+> - if (params.crypto.decrypt || params.crypto.verify) {\r
+> -#ifdef GMIME_ATLEAST_26\r
+> - /* TODO: GMimePasswordRequestFunc */\r
+> - params.crypto.gpgctx = g_mime_gpg_context_new (NULL, "gpg");\r
+> -#else\r
+> - GMimeSession* session = g_object_new (g_mime_session_get_type(), NULL);\r
+> - params.crypto.gpgctx = g_mime_gpg_context_new (session, "gpg");\r
+> -#endif\r
+> - if (params.crypto.gpgctx) {\r
+> - g_mime_gpg_context_set_always_trust ((GMimeGpgContext*) params.crypto.gpgctx, FALSE);\r
+> - } else {\r
+> - /* If we fail to create the gpgctx set the verify and\r
+> - * decrypt flags to FALSE so we don't try to do any\r
+> - * further verification or decryption */\r
+> - params.crypto.verify = FALSE;\r
+> - params.crypto.decrypt = FALSE;\r
+> - fprintf (stderr, "Failed to construct gpg context.\n");\r
+> - }\r
+> -#ifndef GMIME_ATLEAST_26\r
+> - g_object_unref (session);\r
+> -#endif\r
+> - }\r
+> -\r
+> config = notmuch_config_open (ctx, NULL, NULL);\r
+> if (config == NULL)\r
+> return 1;\r
projects / notmuch-archives.git / commitdiff