10/1b33db0f197b211affcd5595a2b1beb5cf340d

   1 Return-Path: <schnouki@schnouki.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 64B9F429E35\r
   6         for <notmuch@notmuchmail.org>; Tue, 17 Jan 2012 02:48:25 -0800 (PST)\r
   7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
   8 X-Spam-Flag: NO\r
   9 X-Spam-Score: -0.1\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=-0.1 tagged_above=-999 required=5\r
  12         tests=[DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1]\r
  13         autolearn=disabled\r
  14 Received: from olra.theworths.org ([127.0.0.1])\r
  15         by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
  16         with ESMTP id bzRiSdjLvwoe for <notmuch@notmuchmail.org>;\r
  17         Tue, 17 Jan 2012 02:48:21 -0800 (PST)\r
  18 Received: from ks3536.kimsufi.com (schnouki.net [87.98.217.222])\r
  19         by olra.theworths.org (Postfix) with ESMTP id F1B68429E2F\r
  20         for <notmuch@notmuchmail.org>; Tue, 17 Jan 2012 02:48:20 -0800 (PST)\r
  21 Received: from thor.loria.fr (thor.loria.fr [152.81.12.250])\r
  22         by ks3536.kimsufi.com (Postfix) with ESMTPSA id B08EB6A002A;\r
  23         Tue, 17 Jan 2012 11:48:19 +0100 (CET)\r
  24 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=schnouki.net;\r
  25         s=key-schnouki; t=1326797299;\r
  26         bh=RW5POdAawFjr6Sr5y28ifptPsjRz3DPD4waqFr1E2vE=;\r
  27         h=From:To:Cc:Subject:In-Reply-To:References:Date:Message-ID:\r
  28         MIME-Version:Content-Type;\r
  29         b=NPBQD1ORBr39OCWBkKL1MER0urzcL4Tq2XzFyzE4PnA+SD2P6t3IRE2QNTGEiIs6O\r
  30         GaOutkrKkd0VcAag6nXXOjh7JLRSTYzjbPSxlRA4kOYLAaL3Tj5a7/GCxyYxlQKP+o\r
  31         y5+c/ziAK/3h5BnSE3eSw2fJpuoawdxG3tIi//sg=\r
  32 From: Thomas Jost <schnouki@schnouki.net>\r
  33 To: Austin Clements <amdragon@MIT.EDU>\r
  34 Subject: Re: [PATCH] Add pseudo-compatibility with gmime 2.6\r
  35 In-Reply-To: <20120117034714.GG16740@mit.edu>\r
  36 References: <8762gbtd6p.fsf@schnouki.net>\r
  37         <1326758199-18058-1-git-send-email-schnouki@schnouki.net>\r
  38         <20120117034714.GG16740@mit.edu>\r
  39 User-Agent: Notmuch/0.11+81~g9729eac (http://notmuchmail.org) Emacs/24.0.92.1\r
  40         (x86_64-unknown-linux-gnu)\r
  41 Date: Tue, 17 Jan 2012 11:48:15 +0100\r
  42 Message-ID: <8739bea9lc.fsf@thor.loria.fr>\r
  43 MIME-Version: 1.0\r
  44 Content-Type: multipart/signed; boundary="=-=-=";\r
  45         micalg=pgp-sha1; protocol="application/pgp-signature"\r
  46 Cc: notmuch@notmuchmail.org\r
  47 X-BeenThere: notmuch@notmuchmail.org\r
  48 X-Mailman-Version: 2.1.13\r
  49 Precedence: list\r
  50 List-Id: "Use and development of the notmuch mail system."\r
  51         <notmuch.notmuchmail.org>\r
  52 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  53         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  54 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  55 List-Post: <mailto:notmuch@notmuchmail.org>\r
  56 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  57 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  58         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  59 X-List-Received-Date: Tue, 17 Jan 2012 10:48:25 -0000\r
  60 \r
  61 --=-=-=\r
  62 Content-Type: text/plain\r
  63 Content-Transfer-Encoding: quoted-printable\r
  64 \r
  65 On Mon, 16 Jan 2012 22:47:14 -0500, Austin Clements <amdragon@MIT.EDU> wrot=\r
  66 e:\r
  67 > Quoth Thomas Jost on Jan 17 at 12:56 am:\r
  68 > > There are lots of API changes in gmime 2.6 crypto handling. By adding\r
  69 > > preprocessor directives, it is however possible to add gmime 2.6 compat=\r
  70 ibility\r
  71 > > while preserving compatibility with gmime 2.4 too.\r
  72 >=20\r
  73 > Awesome.  Comments inline below.\r
  74 \r
  75 Thanks for reviewing this so quickly. Replies inline, and new patches\r
  76 incoming soon.\r
  77 \r
  78 \r
  79 > > This is mostly based on id:"8762i8hrb9.fsf@bookbinder.fernseed.info".\r
  80 > >=20\r
  81 > > This was tested against both gmime 2.6.4 and 2.4.31. With gmime 2.4.31,=\r
  82  the\r
  83 > > crypto tests all work fine (as expected). With gmime 2.6.4, one crypto =\r
  84 test\r
  85 > > fails (signature verification with signer key unavailable) but this wil=\r
  86 l be hard\r
  87 > > to fix since the new API does not report the reason why a signature ver=\r
  88 ification\r
  89 > > fails (other than the human-readable error message).\r
  90 >=20\r
  91 > What is the result of this failing test?\r
  92 \r
  93 The test looks like that:\r
  94 \r
  95         FAIL   signature verification with signer key unavailable\r
  96            --- crypto.4.expected   2012-01-16 23:05:06.765651183 +0000\r
  97            +++ crypto.4.output     2012-01-16 23:05:06.765651183 +0000\r
  98            @@ -12,9 +12,7 @@\r
  99              "Bcc": "",\r
 100              "Date": "01 Jan 2000 12:00:00 -0000"},\r
 101              "body": [{"id": 1,\r
 102            - "sigstatus": [{"status": "error",\r
 103            - "keyid": "6D92612D94E46381",\r
 104            - "errors": 2}],\r
 105            + "sigstatus": [],\r
 106              "content-type": "multipart/signed",\r
 107              "content": [{"id": 2,\r
 108              "content-type": "text/plain",\r
 109        Failed to verify signed part: gpg: keyblock resource `/home/schnouki=\r
 110 /dev/notmuch/test/tmp.crypto/gnupg/pubring.gpg': file open error\r
 111        gpg: armor header: Version: GnuPG v1.4.11 (GNU/Linux)\r
 112        gpg: Signature made Mon Jan 16 23:05:06 2012 UTC using RSA key ID 94=\r
 113 E46381\r
 114        gpg: Can't check signature: public key not found\r
 115 \r
 116 So basically if a signer public key is missing, we can't get the status\r
 117 signature: empty "sigstatus" field in the JSON output, "Unknown\r
 118 signature status" in Emacs.\r
 119 \r
 120 IMHO this is a bug in gmime 2.6, and I think I know what causes it. I'll\r
 121 file a bug in gmime and hopefully they'll find a cleaner way to fix it\r
 122 than the one I came up with :)\r
 123 \r
 124 >=20\r
 125 > > ---\r
 126 > >  mime-node.c      |   50 ++++++++++++++++++++++++++-\r
 127 > >  notmuch-client.h |   27 ++++++++++++++-\r
 128 > >  notmuch-reply.c  |    7 ++++\r
 129 > >  notmuch-show.c   |   97 ++++++++++++++++++++++++++++++++++++++++++++++=\r
 130 ++++++++\r
 131 > >  show-message.c   |    4 ++\r
 132 > >  5 files changed, 181 insertions(+), 4 deletions(-)\r
 133 > >=20\r
 134 > > diff --git a/mime-node.c b/mime-node.c\r
 135 > > index d26bb44..ae2473d 100644\r
 136 > > --- a/mime-node.c\r
 137 > > +++ b/mime-node.c\r
 138 > > @@ -33,7 +33,11 @@ typedef struct mime_node_context {\r
 139 > >      GMimeMessage *mime_message;\r
 140 > >=20=20\r
 141 > >      /* Context provided by the caller. */\r
 142 > > +#ifdef GMIME_26\r
 143 > > +    GMimeCryptoContext *cryptoctx;\r
 144 > > +#else\r
 145 > >      GMimeCipherContext *cryptoctx;\r
 146 > > +#endif\r
 147 > >      notmuch_bool_t decrypt;\r
 148 > >  } mime_node_context_t;\r
 149 > >=20=20\r
 150 > > @@ -57,8 +61,12 @@ _mime_node_context_free (mime_node_context_t *res)\r
 151 > >=20=20\r
 152 > >  notmuch_status_t\r
 153 > >  mime_node_open (const void *ctx, notmuch_message_t *message,\r
 154 > > -           GMimeCipherContext *cryptoctx, notmuch_bool_t decrypt,\r
 155 > > -           mime_node_t **root_out)\r
 156 > > +#ifdef GMIME_26\r
 157 > > +           GMimeCryptoContext *cryptoctx,\r
 158 > > +#else\r
 159 > > +           GMimeCipherContext *cryptoctx,\r
 160 > > +#endif\r
 161 > > +           notmuch_bool_t decrypt, mime_node_t **root_out)\r
 162 > >  {\r
 163 > >      const char *filename =3D notmuch_message_get_filename (message);\r
 164 > >      mime_node_context_t *mctx;\r
 165 > > @@ -112,12 +120,21 @@ DONE:\r
 166 > >      return status;\r
 167 > >  }\r
 168 > >=20=20\r
 169 > > +#ifdef GMIME_26\r
 170 > > +static int\r
 171 > > +_signature_list_free (GMimeSignatureList **proxy)\r
 172 > > +{\r
 173 > > +    g_object_unref (*proxy);\r
 174 > > +    return 0;\r
 175 > > +}\r
 176 > > +#else\r
 177 > >  static int\r
 178 > >  _signature_validity_free (GMimeSignatureValidity **proxy)\r
 179 > >  {\r
 180 > >      g_mime_signature_validity_free (*proxy);\r
 181 > >      return 0;\r
 182 > >  }\r
 183 > > +#endif\r
 184 > >=20=20\r
 185 > >  static mime_node_t *\r
 186 > >  _mime_node_create (const mime_node_t *parent, GMimeObject *part)\r
 187 > > @@ -165,11 +182,23 @@ _mime_node_create (const mime_node_t *parent, GMi=\r
 188 meObject *part)\r
 189 > >         GMimeMultipartEncrypted *encrypteddata =3D\r
 190 > >             GMIME_MULTIPART_ENCRYPTED (part);\r
 191 > >         node->decrypt_attempted =3D TRUE;\r
 192 > > +#ifdef GMIME_26\r
 193 > > +       GMimeDecryptResult *decrypt_result =3D g_mime_decrypt_result_new =\r
 194 ();\r
 195 >=20\r
 196 > I think g_mime_multipart_encrypted_decrypt allocates the\r
 197 > GMimeDecryptResult for you, so this will just leak memory.\r
 198 \r
 199 You're right, fixed. Will it be freed along with node->decrypted_child,\r
 200 or do we need to handle this ourself?\r
 201 \r
 202 >=20\r
 203 > > +       node->decrypted_child =3D g_mime_multipart_encrypted_decrypt\r
 204 > > +           (encrypteddata, node->ctx->cryptoctx, &decrypt_result, &err);\r
 205 > > +       if (node->decrypted_child) {\r
 206 > > +           node->decrypt_success =3D node->verify_attempted =3D TRUE;\r
 207 > > +           node->sig_list =3D g_mime_decrypt_result_get_signatures (decrypt_res=\r
 208 ult);\r
 209 > > +           if (!node->sig_list)\r
 210 > > +               fprintf (stderr, "Failed to get signatures: %s\n",\r
 211 > > +                        (err ? err->message : "no error explanation given"));\r
 212 >=20\r
 213 > My understanding is that g_mime_decrypt_result_get_signatures returns\r
 214 > NULL if there are no signatures and that this isn't an error.  This\r
 215 > differs from 2.4, which would return an empty but non-NULL list.\r
 216 >\r
 217 > Also, I believe you have to free the sig_list in both branches now,\r
 218 > which means the talloc_set_destructor can be moved to common logic\r
 219 > outside of the if decrypted/signed.\r
 220 \r
 221 Hmmm, you're right about g_mime_decrypt_result_get_signatures. I should\r
 222 have RTFM.\r
 223 Also moved talloc_set_destructor outside the if/else, thanks.\r
 224 \r
 225 >=20\r
 226 > > +#else\r
 227 > >         node->decrypted_child =3D g_mime_multipart_encrypted_decrypt\r
 228 > >             (encrypteddata, node->ctx->cryptoctx, &err);\r
 229 > >         if (node->decrypted_child) {\r
 230 > >             node->decrypt_success =3D node->verify_attempted =3D TRUE;\r
 231 > >             node->sig_validity =3D g_mime_multipart_encrypted_get_signature_vali=\r
 232 dity (encrypteddata);\r
 233 > > +#endif\r
 234 >=20\r
 235 > It's confusing to have the open braces in the #ifdef'd region with a\r
 236 > matching close brace outside of it (and I imagine this confuses\r
 237 > editors and uncrustify, too).  You could either copy the else part in\r
 238 > both branches of the #ifdef or avoid duplicated code with something\r
 239 > like\r
 240 >=20\r
 241 > #ifdef GMIME_26\r
 242 >   .. node->decrypted_child =3D ..\r
 243 > #else\r
 244 >   .. node->decrypted_child =3D ..\r
 245 > #endif\r
 246 >   if (node->decrypted_child) {\r
 247 >     node->decrypt_success =3D node->verify_attempted =3D TRUE;\r
 248 > #ifdef GMIME_26\r
 249 >     node->sig_list =3D ..\r
 250 > #else\r
 251 >     node->sig_validity =3D ..\r
 252 > #endif\r
 253 >   } else {\r
 254 >     fprintf (stderr, ..);\r
 255 >   }\r
 256 \r
 257 Right. Avoids duplication and makes it easier to read.\r
 258 \r
 259 >=20\r
 260 > >         } else {\r
 261 > >             fprintf (stderr, "Failed to decrypt part: %s\n",\r
 262 > >                      (err ? err->message : "no error explanation given"));\r
 263 > > @@ -182,6 +211,18 @@ _mime_node_create (const mime_node_t *parent, GMim=\r
 264 eObject *part)\r
 265 > >                  "(must be exactly 2)\n",\r
 266 > >                  node->nchildren);\r
 267 > >     } else {\r
 268 > > +#ifdef GMIME_26\r
 269 > > +       GMimeSignatureList *sig_list =3D g_mime_multipart_signed_verify\r
 270 > > +           (GMIME_MULTIPART_SIGNED (part), node->ctx->cryptoctx, &err);\r
 271 > > +       node->verify_attempted =3D TRUE;\r
 272 > > +       node->sig_list =3D sig_list;\r
 273 > > +       if (sig_list) {\r
 274 > > +           GMimeSignatureList **proxy =3D\r
 275 > > +               talloc (node, GMimeSignatureList *);\r
 276 > > +           *proxy =3D sig_list;\r
 277 > > +           talloc_set_destructor (proxy, _signature_list_free);\r
 278 > > +       }\r
 279 > > +#else\r
 280 > >         /* For some reason the GMimeSignatureValidity returned\r
 281 > >          * here is not a const (inconsistent with that\r
 282 > >          * returned by\r
 283 > > @@ -200,10 +241,15 @@ _mime_node_create (const mime_node_t *parent, GMi=\r
 284 meObject *part)\r
 285 > >             *proxy =3D sig_validity;\r
 286 > >             talloc_set_destructor (proxy, _signature_validity_free);\r
 287 > >         }\r
 288 > > +#endif\r
 289 > >     }\r
 290 > >      }\r
 291 > >=20=20\r
 292 > > +#ifdef GMIME_26\r
 293 > > +    if (node->verify_attempted && !node->sig_list)\r
 294 >=20\r
 295 > Hmm.  This is correct for signed parts, but will incorrectly trigger\r
 296 > for an encrypted part with no signatures.  For 2.6, I think this error\r
 297 > checking may have to move into the branches of the if encrypted/signed\r
 298 > since for encrypted parts you have to check if\r
 299 > g_mime_multipart_encrypted_decrypt returned NULL.\r
 300 \r
 301 That sound right. The weird part is that it did not cause anything to\r
 302 fail in the test suite...\r
 303 \r
 304 Anyway, for 2.6 I moved this test into the "if signed" branch since it's\r
 305 perfectly acceptable to have sig_list =3D=3D NULL for an encrypted part.\r
 306 \r
 307 >=20\r
 308 > > +#else\r
 309 > >      if (node->verify_attempted && !node->sig_validity)\r
 310 > > +#endif\r
 311 > >     fprintf (stderr, "Failed to verify signed part: %s\n",\r
 312 > >              (err ? err->message : "no error explanation given"));\r
 313 > >=20=20\r
 314 > > diff --git a/notmuch-client.h b/notmuch-client.h\r
 315 > > index 517c010..e85f882 100644\r
 316 > > --- a/notmuch-client.h\r
 317 > > +++ b/notmuch-client.h\r
 318 > > @@ -30,6 +30,12 @@\r
 319 > >=20=20\r
 320 > >  #include <gmime/gmime.h>\r
 321 > >=20=20\r
 322 > > +/* GMIME_CHECK_VERSION is only available in gmime >=3D 2.5. But so are\r
 323 > > + * GMIME_MAJOR_VERSION and friends. */\r
 324 >=20\r
 325 > Hah.\r
 326 >=20\r
 327 > > +#ifdef GMIME_MAJOR_VERSION\r
 328 > > +#define GMIME_26\r
 329 > > +#endif\r
 330 > > +\r
 331 > >  #include "notmuch.h"\r
 332 > >=20=20\r
 333 > >  /* This is separate from notmuch-private.h because we're trying to\r
 334 > > @@ -69,7 +75,11 @@ typedef struct notmuch_show_format {\r
 335 > >      void (*part_start) (GMimeObject *part,\r
 336 > >                     int *part_count);\r
 337 > >      void (*part_encstatus) (int status);\r
 338 > > +#ifdef GMIME_26\r
 339 > > +    void (*part_sigstatus) (GMimeSignatureList* siglist);\r
 340 > > +#else\r
 341 > >      void (*part_sigstatus) (const GMimeSignatureValidity* validity);\r
 342 > > +#endif\r
 343 > >      void (*part_content) (GMimeObject *part);\r
 344 > >      void (*part_end) (GMimeObject *part);\r
 345 > >      const char *part_sep;\r
 346 > > @@ -83,7 +93,11 @@ typedef struct notmuch_show_params {\r
 347 > >      int entire_thread;\r
 348 > >      int raw;\r
 349 > >      int part;\r
 350 > > +#ifdef GMIME_26\r
 351 > > +    GMimeCryptoContext* cryptoctx;\r
 352 > > +#else\r
 353 > >      GMimeCipherContext* cryptoctx;\r
 354 > > +#endif\r
 355 > >      int decrypt;\r
 356 > >  } notmuch_show_params_t;\r
 357 > >=20=20\r
 358 > > @@ -286,7 +300,12 @@ typedef struct mime_node {\r
 359 > >       * signature.  May be NULL if signature verification failed.  If\r
 360 > >       * there are simply no signatures, this will be non-NULL with an\r
 361 > >       * empty signers list. */\r
 362 > > +#ifdef GMIME_26\r
 363 > > +    /* TODO: update the above comment... */\r
 364 >=20\r
 365 > Since this behaves very differently in 2.6, I think documenting it is\r
 366 > important (and being very careful about the differences).  Maybe\r
 367 >=20\r
 368 > /* The list of signatures for signed or encrypted containers.  If\r
 369 >  * there are no signatures, this will be NULL. */\r
 370 \r
 371 Added, thanks.\r
 372 \r
 373 >=20\r
 374 > > +    GMimeSignatureList* sig_list;\r
 375 > > +#else\r
 376 > >      const GMimeSignatureValidity *sig_validity;\r
 377 > > +#endif\r
 378 > >=20=20\r
 379 > >      /* Internal: Context inherited from the root iterator. */\r
 380 > >      struct mime_node_context *ctx;\r
 381 > > @@ -311,8 +330,12 @@ typedef struct mime_node {\r
 382 > >   */\r
 383 > >  notmuch_status_t\r
 384 > >  mime_node_open (const void *ctx, notmuch_message_t *message,\r
 385 > > -           GMimeCipherContext *cryptoctx, notmuch_bool_t decrypt,\r
 386 > > -           mime_node_t **node_out);\r
 387 > > +#ifdef GMIME_26\r
 388 > > +           GMimeCryptoContext *cryptoctx,\r
 389 > > +#else\r
 390 > > +           GMimeCipherContext *cryptoctx,\r
 391 > > +#endif\r
 392 > > +           notmuch_bool_t decrypt, mime_node_t **node_out);\r
 393 > >=20=20\r
 394 > >  /* Return a new MIME node for the requested child part of parent.\r
 395 > >   * parent will be used as the talloc context for the returned child\r
 396 > > diff --git a/notmuch-reply.c b/notmuch-reply.c\r
 397 > > index da3acce..dc37c51 100644\r
 398 > > --- a/notmuch-reply.c\r
 399 > > +++ b/notmuch-reply.c\r
 400 > > @@ -688,15 +688,22 @@ notmuch_reply_command (void *ctx, int argc, char =\r
 401 *argv[])\r
 402 > >     reply_format_func =3D notmuch_reply_format_default;\r
 403 > >=20=20\r
 404 > >      if (decrypt) {\r
 405 > > +#ifdef GMIME_26\r
 406 > > +   /* TODO: GMimePasswordRequestFunc */\r
 407 > > +   params.cryptoctx =3D g_mime_gpg_context_new (NULL, "gpg");\r
 408 > > +#else\r
 409 > >     GMimeSession* session =3D g_object_new (g_mime_session_get_type(), NU=\r
 410 LL);\r
 411 > >     params.cryptoctx =3D g_mime_gpg_context_new (session, "gpg");\r
 412 > > +#endif\r
 413 > >     if (params.cryptoctx) {\r
 414 > >         g_mime_gpg_context_set_always_trust ((GMimeGpgContext*) params.cr=\r
 415 yptoctx, FALSE);\r
 416 > >         params.decrypt =3D TRUE;\r
 417 > >     } else {\r
 418 > >         fprintf (stderr, "Failed to construct gpg context.\n");\r
 419 > >     }\r
 420 > > +#ifndef GMIME_26\r
 421 > >     g_object_unref (session);\r
 422 > > +#endif\r
 423 > >      }\r
 424 > >=20=20\r
 425 > >      config =3D notmuch_config_open (ctx, NULL, NULL);\r
 426 > > diff --git a/notmuch-show.c b/notmuch-show.c\r
 427 > > index d14dac9..263ab72 100644\r
 428 > > --- a/notmuch-show.c\r
 429 > > +++ b/notmuch-show.c\r
 430 > > @@ -76,7 +76,11 @@ static void\r
 431 > >  format_part_encstatus_json (int status);\r
 432 > >=20=20\r
 433 > >  static void\r
 434 > > +#ifdef GMIME_26\r
 435 > > +format_part_sigstatus_json (GMimeSignatureList* siglist);\r
 436 > > +#else\r
 437 > >  format_part_sigstatus_json (const GMimeSignatureValidity* validity);\r
 438 > > +#endif\r
 439 > >=20=20\r
 440 > >  static void\r
 441 > >  format_part_content_json (GMimeObject *part);\r
 442 > > @@ -486,6 +490,21 @@ show_text_part_content (GMimeObject *part, GMimeSt=\r
 443 ream *stream_out)\r
 444 > >     g_object_unref(stream_filter);\r
 445 > >  }\r
 446 > >=20=20\r
 447 > > +#ifdef GMIME_26\r
 448 > > +static const char*\r
 449 > > +signature_status_to_string (GMimeSignatureStatus x)\r
 450 > > +{\r
 451 > > +    switch (x) {\r
 452 > > +    case GMIME_SIGNATURE_STATUS_GOOD:\r
 453 > > +   return "good";\r
 454 > > +    case GMIME_SIGNATURE_STATUS_BAD:\r
 455 > > +   return "bad";\r
 456 > > +    case GMIME_SIGNATURE_STATUS_ERROR:\r
 457 > > +   return "error";\r
 458 > > +    }\r
 459 > > +    return "unknown";\r
 460 > > +}\r
 461 > > +#else\r
 462 > >  static const char*\r
 463 > >  signer_status_to_string (GMimeSignerStatus x)\r
 464 > >  {\r
 465 > > @@ -501,6 +520,7 @@ signer_status_to_string (GMimeSignerStatus x)\r
 466 > >      }\r
 467 > >      return "unknown";\r
 468 > >  }\r
 469 > > +#endif\r
 470 > >=20=20\r
 471 > >  static void\r
 472 > >  format_part_start_text (GMimeObject *part, int *part_count)\r
 473 > > @@ -592,6 +612,75 @@ format_part_encstatus_json (int status)\r
 474 > >      printf ("}]");\r
 475 > >  }\r
 476 > >=20=20\r
 477 > > +#ifdef GMIME_26\r
 478 > > +static void\r
 479 > > +format_part_sigstatus_json (GMimeSignatureList *siglist)\r
 480 > > +{\r
 481 > > +    printf (", \"sigstatus\": [");\r
 482 > > +\r
 483 > > +    if (!siglist) {\r
 484 > > +   printf ("]");\r
 485 > > +   return;\r
 486 > > +     }\r
 487 > > +\r
 488 > > +    void *ctx_quote =3D talloc_new (NULL);\r
 489 > > +    int i;\r
 490 > > +    for (i =3D 0; i < g_mime_signature_list_length (siglist); ++i) {\r
 491 >=20\r
 492 > Style nit: notmuch uses i++.\r
 493 \r
 494 Ack.\r
 495 \r
 496 >=20\r
 497 > > +   GMimeSignature *signature =3D g_mime_signature_list_get_signature (si=\r
 498 glist, i);\r
 499 > > +\r
 500 > > +   if (i > 0)\r
 501 > > +       printf (", ");\r
 502 > > +\r
 503 > > +   printf ("{");\r
 504 > > +\r
 505 > > +   /* status */\r
 506 > > +   GMimeSignatureStatus status =3D g_mime_signature_get_status (signatur=\r
 507 e);\r
 508 > > +   printf ("\"status\": %s",\r
 509 > > +           json_quote_str (ctx_quote,\r
 510 > > +                           signature_status_to_string (status)));\r
 511 > > +\r
 512 > > +   GMimeCertificate *certificate =3D g_mime_signature_get_certificate (s=\r
 513 ignature);\r
 514 > > +   if (status =3D=3D GMIME_SIGNATURE_STATUS_GOOD)\r
 515 > > +   {\r
 516 >=20\r
 517 > Style nit: break after brace.\r
 518 >=20\r
 519 > (Presumably this is copied from the existing\r
 520 > format_part_sigstatus_json, but since it's technically new code\r
 521 > there's no reason not to fix these up.)\r
 522 \r
 523 Ack too. (Copied from the original patch, which was probably copied from\r
 524 the existing code...)\r
 525 \r
 526 >=20\r
 527 > > +       if (certificate)\r
 528 > > +           printf (", \"fingerprint\": %s", json_quote_str (ctx_quote, g_mime_c=\r
 529 ertificate_get_fingerprint (certificate)));\r
 530 > > +       /* these dates are seconds since the epoch; should we\r
 531 > > +        * provide a more human-readable format string? */\r
 532 > > +       time_t created =3D g_mime_signature_get_created (signature);\r
 533 > > +       if (created !=3D -1)\r
 534 > > +           printf (", \"created\": %d", (int) created);\r
 535 > > +       time_t expires =3D g_mime_signature_get_expires (signature);\r
 536 > > +       if (expires > 0)\r
 537 > > +           printf (", \"expires\": %d", (int) expires);\r
 538 >=20\r
 539 > Is it intentional that the two above checks are different?  I would\r
 540 > think the second should be expires !=3D -1.\r
 541 \r
 542 It was so in the original patch, which caused one of the tests to fail.\r
 543 =2D1 means "unknown", and AFAICT 0 means "never expires". The current\r
 544 behaviour is to add the "expires" field only if there is an expiration\r
 545 date, so we need to check if expires > 0.\r
 546 \r
 547 >=20\r
 548 > > +       /* output user id only if validity is FULL or ULTIMATE. */\r
 549 > > +       /* note that gmime is using the term "trust" here, which\r
 550 > > +        * is WRONG.  It's actually user id "validity". */\r
 551 > > +       if (certificate)\r
 552 > > +       {\r
 553 >=20\r
 554 > Break after brace.\r
 555 >=20\r
 556 > > +           const char *name =3D g_mime_certificate_get_name (certificate);\r
 557 > > +           GMimeCertificateTrust trust =3D g_mime_certificate_get_trust (certif=\r
 558 icate);\r
 559 > > +           if (name && (trust =3D=3D GMIME_CERTIFICATE_TRUST_FULLY || trust =3D=\r
 560 =3D GMIME_CERTIFICATE_TRUST_ULTIMATE))\r
 561 > > +               printf (", \"userid\": %s", json_quote_str (ctx_quote, name));\r
 562 > > +       }\r
 563 > > +   } else if (certificate) {\r
 564 > > +       const char *key_id =3D g_mime_certificate_get_key_id (certificate=\r
 565 );\r
 566 > > +       if (key_id)\r
 567 > > +           printf (", \"keyid\": %s", json_quote_str (ctx_quote, key_id));\r
 568 > > +   }\r
 569 > > +\r
 570 > > +   GMimeSignatureError errors =3D g_mime_signature_get_errors (signature=\r
 571 );\r
 572 > > +   if (errors !=3D GMIME_SIGNATURE_ERROR_NONE) {\r
 573 > > +       printf (", \"errors\": %x", errors);\r
 574 >=20\r
 575 > This should be %d (I would say 0x%x, but JSON doesn't support hex\r
 576 > literals).  I see this bug came from the original\r
 577 > format_part_sigstatus_json.  Maybe there should be a quick patch\r
 578 > before this one that fixes the source of the bug?\r
 579 \r
 580 Right. I'll add a first patch to change this in the original format_part_si=\r
 581 gstatus_json.\r
 582 \r
 583 >=20\r
 584 > > +   }\r
 585 > > +\r
 586 > > +   printf ("}");\r
 587 > > +     }\r
 588 > > +\r
 589 > > +    printf ("]");\r
 590 > > +\r
 591 > > +    talloc_free (ctx_quote);\r
 592 > > +}\r
 593 > > +#else\r
 594 > >  static void\r
 595 > >  format_part_sigstatus_json (const GMimeSignatureValidity* validity)\r
 596 > >  {\r
 597 > > @@ -652,6 +741,7 @@ format_part_sigstatus_json (const GMimeSignatureVal=\r
 598 idity* validity)\r
 599 > >=20=20\r
 600 > >      talloc_free (ctx_quote);\r
 601 > >  }\r
 602 > > +#endif\r
 603 > >=20=20\r
 604 > >  static void\r
 605 > >  format_part_content_json (GMimeObject *part)\r
 606 > > @@ -990,13 +1080,20 @@ notmuch_show_command (void *ctx, unused (int arg=\r
 607 c), unused (char *argv[]))\r
 608 > >     } else if ((STRNCMP_LITERAL (argv[i], "--verify") =3D=3D 0) ||\r
 609 > >                (STRNCMP_LITERAL (argv[i], "--decrypt") =3D=3D 0)) {\r
 610 > >         if (params.cryptoctx =3D=3D NULL) {\r
 611 > > +#ifdef GMIME_26\r
 612 > > +           /* TODO: GMimePasswordRequestFunc */\r
 613 > > +           if (NULL =3D=3D (params.cryptoctx =3D g_mime_gpg_context_new(NULL, "=\r
 614 gpg")))\r
 615 > > +#else\r
 616 > >             GMimeSession* session =3D g_object_new(g_mime_session_get_type(), NU=\r
 617 LL);\r
 618 > >             if (NULL =3D=3D (params.cryptoctx =3D g_mime_gpg_context_new(session=\r
 619 , "gpg")))\r
 620 > > +#endif\r
 621 > >                 fprintf (stderr, "Failed to construct gpg context.\n");\r
 622 > >             else\r
 623 > >                 g_mime_gpg_context_set_always_trust((GMimeGpgContext*)params.cry=\r
 624 ptoctx, FALSE);\r
 625 > > +#ifndef GMIME_26\r
 626 > >             g_object_unref (session);\r
 627 > >             session =3D NULL;\r
 628 > > +#endif\r
 629 > >         }\r
 630 > >         if (STRNCMP_LITERAL (argv[i], "--decrypt") =3D=3D 0)\r
 631 > >             params.decrypt =3D 1;\r
 632 > > diff --git a/show-message.c b/show-message.c\r
 633 > > index 8768889..65269fd 100644\r
 634 > > --- a/show-message.c\r
 635 > > +++ b/show-message.c\r
 636 > > @@ -48,7 +48,11 @@ show_message_part (mime_node_t *node,\r
 637 > >     format->part_encstatus (node->decrypt_success);\r
 638 > >=20=20\r
 639 > >      if (node->verify_attempted && format->part_sigstatus)\r
 640 > > +#ifdef GMIME_26\r
 641 > > +   format->part_sigstatus (node->sig_list);\r
 642 > > +#else\r
 643 > >     format->part_sigstatus (node->sig_validity);\r
 644 > > +#endif\r
 645 > >=20=20\r
 646 > >      format->part_content (part);\r
 647 > >=20=20\r
 648 \r
 649 =2D-=20\r
 650 Thomas/Schnouki\r
 651 \r
 652 --=-=-=\r
 653 Content-Type: application/pgp-signature\r
 654 \r
 655 -----BEGIN PGP SIGNATURE-----\r
 656 Version: GnuPG v1.4.11 (GNU/Linux)\r
 657 \r
 658 iQEcBAEBAgAGBQJPFVHvAAoJEMPdciX+bh5I7RYH/283CLnwJtB4Li9Nk+za5Wx6\r
 659 zOC5eAUBA1N/Nt0082Mztdj/RDtRUhH5CuqZM/vxoTGLpFS+r7+sg0vMLVlfWcbd\r
 660 THmMtJOOhf6bfUMgHLNE0mGHFvRlcLRsG4D1/C82X/zu9+I4bPSQ/ODuCGH/gWeF\r
 661 XAM9RlaZrkPes5XDsf3EpsMELVWS4vPXmwuSuAOXAbTFEPTDDrOUZMbcNsjgiAmy\r
 662 MdAr6UgNFsjPs3u8RMKnrdvvIawhQXXhozWp3ZQbbuaK6N8KGfzllFVifvwCdHll\r
 663 MW6WJquhQQZiVDUMOjVH3DKCbNvb5xjAVvK2dqQ6b5ZnsDOO5Tm19x+Man6ltRU=\r
 664 =xvIU\r
 665 -----END PGP SIGNATURE-----\r
 666 --=-=-=--\r