ed/090a9e7ff87fb3da420431774c1b9cbec5cabd

   1 Return-Path: <jrollins@finestructure.net>\r
   2 X-Original-To: notmuch@notmuchmail.org\r
   3 Delivered-To: notmuch@notmuchmail.org\r
   4 Received: from localhost (localhost [127.0.0.1])\r
   5         by olra.theworths.org (Postfix) with ESMTP id 86CB4431FB6\r
   6         for <notmuch@notmuchmail.org>; Fri, 18 May 2012 12:45:16 -0700 (PDT)\r
   7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
   8 X-Spam-Flag: NO\r
   9 X-Spam-Score: -2.29\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=-2.29 tagged_above=-999 required=5\r
  12         tests=[RCVD_IN_DNSWL_MED=-2.3, T_MIME_NO_TEXT=0.01] autolearn=disabled\r
  13 Received: from olra.theworths.org ([127.0.0.1])\r
  14         by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
  15         with ESMTP id rROkj6zrb-U4 for <notmuch@notmuchmail.org>;\r
  16         Fri, 18 May 2012 12:45:15 -0700 (PDT)\r
  17 Received: from outgoing-mail.its.caltech.edu (outgoing-mail.its.caltech.edu\r
  18         [131.215.239.19])\r
  19         by olra.theworths.org (Postfix) with ESMTP id 04677431FAE\r
  20         for <notmuch@notmuchmail.org>; Fri, 18 May 2012 12:45:15 -0700 (PDT)\r
  21 Received: from fire-doxen.imss.caltech.edu (localhost [127.0.0.1])\r
  22         by fire-doxen-postvirus (Postfix) with ESMTP id B78962E507A4;\r
  23         Fri, 18 May 2012 12:45:14 -0700 (PDT)\r
  24 X-Spam-Scanned: at Caltech-IMSS on fire-doxen by amavisd-new\r
  25 Received: from finestructure.net (rrcs-24-103-26-131.nyc.biz.rr.com\r
  26         [24.103.26.131]) (Authenticated sender: jrollins)\r
  27         by fire-doxen-submit (Postfix) with ESMTP id A9C11328075;\r
  28         Fri, 18 May 2012 12:45:11 -0700 (PDT)\r
  29 Received: by finestructure.net (Postfix, from userid 1000)\r
  30         id 893574AD; Fri, 18 May 2012 12:45:10 -0700 (PDT)\r
  31 From: Jameson Graef Rollins <jrollins@finestructure.net>\r
  32 To: Austin Clements <amdragon@MIT.EDU>\r
  33 Subject: Re: [PATCH v2 5/5] cli: lazily create the crypto gpg context only\r
  34         when needed\r
  35 In-Reply-To: <20120518192157.GV11804@mit.edu>\r
  36 References: <1337362357-31281-1-git-send-email-jrollins@finestructure.net>\r
  37         <1337362357-31281-2-git-send-email-jrollins@finestructure.net>\r
  38         <1337362357-31281-3-git-send-email-jrollins@finestructure.net>\r
  39         <1337362357-31281-4-git-send-email-jrollins@finestructure.net>\r
  40         <1337362357-31281-5-git-send-email-jrollins@finestructure.net>\r
  41         <1337362357-31281-6-git-send-email-jrollins@finestructure.net>\r
  42         <20120518192157.GV11804@mit.edu>\r
  43 User-Agent: Notmuch/0.12+183~g9d5ff3c (http://notmuchmail.org) Emacs/23.4.1\r
  44         (x86_64-pc-linux-gnu)\r
  45 Date: Fri, 18 May 2012 12:45:08 -0700\r
  46 Message-ID: <87txzd9su3.fsf@servo.finestructure.net>\r
  47 MIME-Version: 1.0\r
  48 Content-Type: multipart/signed; boundary="=-=-=";\r
  49         micalg=pgp-sha256; protocol="application/pgp-signature"\r
  50 Cc: Notmuch Mail <notmuch@notmuchmail.org>\r
  51 X-BeenThere: notmuch@notmuchmail.org\r
  52 X-Mailman-Version: 2.1.13\r
  53 Precedence: list\r
  54 List-Id: "Use and development of the notmuch mail system."\r
  55         <notmuch.notmuchmail.org>\r
  56 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  57         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  58 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  59 List-Post: <mailto:notmuch@notmuchmail.org>\r
  60 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  61 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  62         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  63 X-List-Received-Date: Fri, 18 May 2012 19:45:16 -0000\r
  64 \r
  65 --=-=-=\r
  66 \r
  67 On Fri, May 18 2012, Austin Clements <amdragon@MIT.EDU> wrote:\r
  68 >> +    /* Lazily create the gpgctx if it's needed and hasn't been initialized yet */\r
  69 >> +    if ((GMIME_IS_MULTIPART_ENCRYPTED (part) || GMIME_IS_MULTIPART_SIGNED (part))\r
  70 >> +    && (node->ctx->crypto->verify || node->ctx->crypto->decrypt)) {\r
  71 >> +    if (!node->ctx->crypto->gpgctx) {\r
  72 >\r
  73 > These if conditions could be combined, like\r
  74 >\r
  75 >     if ((GMIME_IS_MULTIPART_ENCRYPTED (part) || GMIME_IS_MULTIPART_SIGNED (part))\r
  76 >       && (node->ctx->crypto->verify || node->ctx->crypto->decrypt)\r
  77 >       && !node->ctx->crypto->gpgctx) {\r
  78 >\r
  79 > When I see two nested 'if's like this, I expect there to be an else\r
  80 > part to the inner if or something after the inner if (why else would\r
  81 > it be separate?) and then I wind up matching braces when I don't see\r
  82 > anything.  Also, one 'if' would save a level of indentation.\r
  83 \r
  84 To explain what I was explaining on IRC, and what I should have noted in\r
  85 the original patch, I did this on purpose because I'm looking forward to\r
  86 the S/MIME support I was trying to get working.\r
  87 \r
  88 gmime 2.6 provides a separate context for pkcs7 (S/MIME).  In this\r
  89 context initialization section we will therefore have to test for\r
  90 initialization of the relevant context.  Since I knew that going into\r
  91 this I decided to anticipate it by constructing things this way now\r
  92 future diffs wouldn't have to include the indentation of this section\r
  93 and could therefore be cleaner and smaller.  If folks have issue with\r
  94 that explanation let me know.\r
  95 \r
  96 > Perhaps "If crypto->gpgctx is NULL, it will be lazily initialized."?\r
  97 > The variable does have to be "initialized", in the sense that it can't\r
  98 > be uninitialized data.\r
  99 \r
 100 That sounds like a better wording.  I'll fix.\r
 101 \r
 102 > It's slightly awkward that it's the caller's responsibility to free\r
 103 > this lazily constructed object.  That should probably be documented.\r
 104 > We could more carefully reference count it, but I think that would\r
 105 > actually be worse because the reference count would probably bounce\r
 106 > through zero frequently.\r
 107 \r
 108 I agree that this is awkward.  Is there a suggestion on how to do it\r
 109 better?  We only want to initialize it if it's needed, and only\r
 110 _mime_node_create knows that.  And we don't want to free it with\r
 111 _mime_node_context_free, or something, only to have to reinitialize it\r
 112 again with the next node or message.  Thoughts?\r
 113 \r
 114 jamie.\r
 115 \r
 116 --=-=-=\r
 117 Content-Type: application/pgp-signature\r
 118 \r
 119 -----BEGIN PGP SIGNATURE-----\r
 120 Version: GnuPG v1.4.12 (GNU/Linux)\r
 121 \r
 122 iQIcBAEBCAAGBQJPtqbEAAoJEO00zqvie6q8GOYP+wZO99ubAxHOriE9RK5KqZQM\r
 123 ATLxdRhZ9r1AElwTo7sZpyh1uAiprArjN9U7UM5BwZKICfXaaw7kJ9uNdQibdb2B\r
 124 m7DnPUtf4dFkUTHY0G4866evzUX2FehJwVHAmJ61mjlUtSNTqfkvgQ9aCBC0JuUS\r
 125 yNFxLrSFtn4tEI38PCMrG+rkjcJwcGA/6KA5yvipjjLrYSG+YWvvdFA0q26MUC9g\r
 126 oqycy0T4KminjofXckKoiGL/gs32i1u5AV8eORKBXDivbt6twU/aRnUucCIhqrMx\r
 127 RXUZad7OLTG2G/OLTrXs/pNjIl39DJYivzzh8+xLEYeL7EX0Tw85wwxxojLI/hvA\r
 128 xXRayU7R1pA3C8666OyA65uy22/Os/TSWYB2VmHqeIlgAt5I5bUGLaV6sKhN81Ho\r
 129 4QbBAh0Fs976EUdLwKhZsUQ5VqhJk8whB1mmoa9PtQw6/thiNV28BgCkM8DIxWOK\r
 130 DXDVkVmlbmUj93eYNYxqYaTncpQqX1cBEIlO/Mz+xON959kjwYRCS/D8yKXm6rZ9\r
 131 95HpztmcxBawjYhaErPtGwCGFGXBY4fs87Dx+8zLemgXRrNUPoWG/3lPfvBze+t2\r
 132 Ws4P7/9hO8aiqV9MMb7gQLqnE1xTNV9m5cT17aHe8klKVR8a1igyF2e8+3V3lpfH\r
 133 PZuNaT4xDaV9x0/SGvio\r
 134 =IZeH\r
 135 -----END PGP SIGNATURE-----\r
 136 --=-=-=--\r