71/6ea5be1a845465a45272cb6d82527dc085852b

   1 Return-Path: <aperez@igalia.com>\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 47473429E35\r
   6         for <notmuch@notmuchmail.org>; Tue, 17 Jan 2012 02:46:05 -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.7\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5\r
  12         tests=[RCVD_IN_DNSWL_LOW=-0.7] 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 zmvkwppDBWQ0 for <notmuch@notmuchmail.org>;\r
  16         Tue, 17 Jan 2012 02:46:01 -0800 (PST)\r
  17 Received: from smtp4.mundo-r.com (smtp4.mundo-r.com [212.51.32.151])\r
  18         by olra.theworths.org (Postfix) with ESMTP id 381A6429E2F\r
  19         for <notmuch@notmuchmail.org>; Tue, 17 Jan 2012 02:46:00 -0800 (PST)\r
  20 X-IronPort-Anti-Spam-Filtered: true\r
  21 X-IronPort-Anti-Spam-Result: Ap8EAANRFU9bdWOb/2dsb2JhbABCAoURpyuCC4FyAQEFIwQvMwsYCQ0UAgIPASkNERmHfAakVZFiiQcXQgEFCAUEEQUBBgEBBgEFLQIBCQEBAS4KTQ0FGBEKgWkCBgE3EQsBgX6BFgSVEZJD\r
  22 X-IronPort-AV: E=Sophos;i="4.71,522,1320620400"; d="scan'208";a="862337471"\r
  23 Received: from 155.99.117.91.static.mundo-r.com (HELO fanzine.igalia.com)\r
  24         ([91.117.99.155])\r
  25         by smtp4.mundo-r.com with ESMTP; 17 Jan 2012 11:45:58 +0100\r
  26 Received: from dsl-hkibrasgw3-fe7cfb00-20.dhcp.inet.fi ([84.251.124.20]\r
  27         helo=kaze.ara-ara.org) by fanzine.igalia.com with esmtpsa \r
  28         (Cipher TLS1.0:RSA_ARCFOUR_SHA1:16) (Exim) id 1Rn6Y9-0003E3-Vm\r
  29         for <notmuch@notmuchmail.org>; Tue, 17 Jan 2012 11:45:58 +0100\r
  30 Received: (nullmailer pid 975 invoked by uid 1000);\r
  31         Tue, 17 Jan 2012 10:48:18 -0000\r
  32 From: Adrian Perez <aperez@igalia.com>\r
  33 To: notmuch@notmuchmail.org\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 (http://notmuchmail.org) Emacs/23.3.1\r
  40         (x86_64-unknown-linux-gnu)\r
  41 Date: Tue, 17 Jan 2012 12:48:14 +0200\r
  42 Message-ID: <87ty3u38r5.fsf@igalia.com>\r
  43 MIME-Version: 1.0\r
  44 Content-Type: multipart/signed; boundary="=-=-=";\r
  45         micalg=pgp-sha1; protocol="application/pgp-signature"\r
  46 X-BeenThere: notmuch@notmuchmail.org\r
  47 X-Mailman-Version: 2.1.13\r
  48 Precedence: list\r
  49 List-Id: "Use and development of the notmuch mail system."\r
  50         <notmuch.notmuchmail.org>\r
  51 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  52         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  53 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  54 List-Post: <mailto:notmuch@notmuchmail.org>\r
  55 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  56 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  57         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  58 X-List-Received-Date: Tue, 17 Jan 2012 10:46:05 -0000\r
  59 \r
  60 --=-=-=\r
  61 Content-Type: text/plain; charset=utf-8\r
  62 Content-Transfer-Encoding: quoted-printable\r
  63 \r
  64 \r
  65 Hi,=20\r
  66 \r
  67 Just a couple of comments inline :)\r
  68 \r
  69 On Mon, 16 Jan 2012 22:47:14 -0500, Austin Clements <amdragon@MIT.EDU> wrot=\r
  70 e:\r
  71 > Quoth Thomas Jost on Jan 17 at 12:56 am:\r
  72 > > There are lots of API changes in gmime 2.6 crypto handling. By adding\r
  73 > > preprocessor directives, it is however possible to add gmime 2.6 compat=\r
  74 ibility\r
  75 > > while preserving compatibility with gmime 2.4 too.\r
  76 >=20\r
  77 > Awesome.  Comments inline below.\r
  78 >=20\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 >=20\r
  93 > > ---\r
  94 > >  mime-node.c      |   50 ++++++++++++++++++++++++++-\r
  95 > >  notmuch-client.h |   27 ++++++++++++++-\r
  96 > >  notmuch-reply.c  |    7 ++++\r
  97 > >  notmuch-show.c   |   97 ++++++++++++++++++++++++++++++++++++++++++++++=\r
  98 ++++++++\r
  99 > >  show-message.c   |    4 ++\r
 100 > >  5 files changed, 181 insertions(+), 4 deletions(-)\r
 101 > >=20\r
 102 > > diff --git a/mime-node.c b/mime-node.c\r
 103 > > index d26bb44..ae2473d 100644\r
 104 > > --- a/mime-node.c\r
 105 > > +++ b/mime-node.c\r
 106 > > @@ -33,7 +33,11 @@ typedef struct mime_node_context {\r
 107 > >      GMimeMessage *mime_message;\r
 108 > >=20=20\r
 109 > >      /* Context provided by the caller. */\r
 110 > > +#ifdef GMIME_26\r
 111 > > +    GMimeCryptoContext *cryptoctx;\r
 112 > > +#else\r
 113 > >      GMimeCipherContext *cryptoctx;\r
 114 > > +#endif\r
 115 > >      notmuch_bool_t decrypt;\r
 116 > >  } mime_node_context_t;\r
 117 > >=20=20\r
 118 > > @@ -57,8 +61,12 @@ _mime_node_context_free (mime_node_context_t *res)\r
 119 > >=20=20\r
 120 > >  notmuch_status_t\r
 121 > >  mime_node_open (const void *ctx, notmuch_message_t *message,\r
 122 > > -           GMimeCipherContext *cryptoctx, notmuch_bool_t decrypt,\r
 123 > > -           mime_node_t **root_out)\r
 124 > > +#ifdef GMIME_26\r
 125 > > +           GMimeCryptoContext *cryptoctx,\r
 126 > > +#else\r
 127 > > +           GMimeCipherContext *cryptoctx,\r
 128 > > +#endif\r
 129 > > +           notmuch_bool_t decrypt, mime_node_t **root_out)\r
 130 > >  {\r
 131 > >      const char *filename =3D notmuch_message_get_filename (message);\r
 132 > >      mime_node_context_t *mctx;\r
 133 > > @@ -112,12 +120,21 @@ DONE:\r
 134 > >      return status;\r
 135 > >  }\r
 136 > >=20=20\r
 137 > > +#ifdef GMIME_26\r
 138 > > +static int\r
 139 > > +_signature_list_free (GMimeSignatureList **proxy)\r
 140 > > +{\r
 141 > > +    g_object_unref (*proxy);\r
 142 > > +    return 0;\r
 143 > > +}\r
 144 > > +#else\r
 145 > >  static int\r
 146 > >  _signature_validity_free (GMimeSignatureValidity **proxy)\r
 147 > >  {\r
 148 > >      g_mime_signature_validity_free (*proxy);\r
 149 > >      return 0;\r
 150 > >  }\r
 151 > > +#endif\r
 152 > >=20=20\r
 153 > >  static mime_node_t *\r
 154 > >  _mime_node_create (const mime_node_t *parent, GMimeObject *part)\r
 155 > > @@ -165,11 +182,23 @@ _mime_node_create (const mime_node_t *parent, GMi=\r
 156 meObject *part)\r
 157 > >         GMimeMultipartEncrypted *encrypteddata =3D\r
 158 > >             GMIME_MULTIPART_ENCRYPTED (part);\r
 159 > >         node->decrypt_attempted =3D TRUE;\r
 160 > > +#ifdef GMIME_26\r
 161 > > +       GMimeDecryptResult *decrypt_result =3D g_mime_decrypt_result_new =\r
 162 ();\r
 163 >=20\r
 164 > I think g_mime_multipart_encrypted_decrypt allocates the\r
 165 > GMimeDecryptResult for you, so this will just leak memory.\r
 166 \r
 167 Yes, this is a leak. The documentation says that the function is\r
 168 responsible of allocating the GMimeDecryptResult. The g_object_unref()\r
 169 is still needed in notmuch's code to free it, though.\r
 170 \r
 171 See http://git.gnome.org/browse/gmime/tree/gmime/gmime-multipart-encrypted.=\r
 172 c#n282\r
 173 \r
 174 > > +       node->decrypted_child =3D g_mime_multipart_encrypted_decrypt\r
 175 > > +           (encrypteddata, node->ctx->cryptoctx, &decrypt_result, &err);\r
 176 > > +       if (node->decrypted_child) {\r
 177 > > +           node->decrypt_success =3D node->verify_attempted =3D TRUE;\r
 178 > > +           node->sig_list =3D g_mime_decrypt_result_get_signatures (decrypt_res=\r
 179 ult);\r
 180 > > +           if (!node->sig_list)\r
 181 > > +               fprintf (stderr, "Failed to get signatures: %s\n",\r
 182 > > +                        (err ? err->message : "no error explanation given"));\r
 183 >=20\r
 184 > My understanding is that g_mime_decrypt_result_get_signatures returns\r
 185 > NULL if there are no signatures and that this isn't an error.  This\r
 186 > differs from 2.4, which would return an empty but non-NULL list.\r
 187 >=20\r
 188 > Also, I believe you have to free the sig_list in both branches now,\r
 189 > which means the talloc_set_destructor can be moved to common logic\r
 190 > outside of the if decrypted/signed.\r
 191 >=20\r
 192 > > +#else\r
 193 > >         node->decrypted_child =3D g_mime_multipart_encrypted_decrypt\r
 194 > >             (encrypteddata, node->ctx->cryptoctx, &err);\r
 195 > >         if (node->decrypted_child) {\r
 196 > >             node->decrypt_success =3D node->verify_attempted =3D TRUE;\r
 197 > >             node->sig_validity =3D g_mime_multipart_encrypted_get_signature_vali=\r
 198 dity (encrypteddata);\r
 199 > > +#endif\r
 200 >=20\r
 201 > It's confusing to have the open braces in the #ifdef'd region with a\r
 202 > matching close brace outside of it (and I imagine this confuses\r
 203 > editors and uncrustify, too).  You could either copy the else part in\r
 204 > both branches of the #ifdef or avoid duplicated code with something\r
 205 > like\r
 206 >=20\r
 207 > #ifdef GMIME_26\r
 208 >   .. node->decrypted_child =3D ..\r
 209 > #else\r
 210 >   .. node->decrypted_child =3D ..\r
 211 > #endif\r
 212 >   if (node->decrypted_child) {\r
 213 >     node->decrypt_success =3D node->verify_attempted =3D TRUE;\r
 214 > #ifdef GMIME_26\r
 215 >     node->sig_list =3D ..\r
 216 > #else\r
 217 >     node->sig_validity =3D ..\r
 218 > #endif\r
 219 >   } else {\r
 220 >     fprintf (stderr, ..);\r
 221 >   }\r
 222 >=20\r
 223 > >         } else {\r
 224 > >             fprintf (stderr, "Failed to decrypt part: %s\n",\r
 225 > >                      (err ? err->message : "no error explanation given"));\r
 226 > > @@ -182,6 +211,18 @@ _mime_node_create (const mime_node_t *parent, GMim=\r
 227 eObject *part)\r
 228 > >                  "(must be exactly 2)\n",\r
 229 > >                  node->nchildren);\r
 230 > >     } else {\r
 231 > > +#ifdef GMIME_26\r
 232 > > +       GMimeSignatureList *sig_list =3D g_mime_multipart_signed_verify\r
 233 > > +           (GMIME_MULTIPART_SIGNED (part), node->ctx->cryptoctx, &err);\r
 234 > > +       node->verify_attempted =3D TRUE;\r
 235 > > +       node->sig_list =3D sig_list;\r
 236 > > +       if (sig_list) {\r
 237 > > +           GMimeSignatureList **proxy =3D\r
 238 > > +               talloc (node, GMimeSignatureList *);\r
 239 > > +           *proxy =3D sig_list;\r
 240 > > +           talloc_set_destructor (proxy, _signature_list_free);\r
 241 > > +       }\r
 242 > > +#else\r
 243 > >         /* For some reason the GMimeSignatureValidity returned\r
 244 > >          * here is not a const (inconsistent with that\r
 245 > >          * returned by\r
 246 > > @@ -200,10 +241,15 @@ _mime_node_create (const mime_node_t *parent, GMi=\r
 247 meObject *part)\r
 248 > >             *proxy =3D sig_validity;\r
 249 > >             talloc_set_destructor (proxy, _signature_validity_free);\r
 250 > >         }\r
 251 > > +#endif\r
 252 > >     }\r
 253 > >      }\r
 254 > >=20=20\r
 255 > > +#ifdef GMIME_26\r
 256 > > +    if (node->verify_attempted && !node->sig_list)\r
 257 >=20\r
 258 > Hmm.  This is correct for signed parts, but will incorrectly trigger\r
 259 > for an encrypted part with no signatures.  For 2.6, I think this error\r
 260 > checking may have to move into the branches of the if encrypted/signed\r
 261 > since for encrypted parts you have to check if\r
 262 > g_mime_multipart_encrypted_decrypt returned NULL.\r
 263 >=20\r
 264 > > +#else\r
 265 > >      if (node->verify_attempted && !node->sig_validity)\r
 266 > > +#endif\r
 267 > >     fprintf (stderr, "Failed to verify signed part: %s\n",\r
 268 > >              (err ? err->message : "no error explanation given"));\r
 269 > >=20=20\r
 270 > > diff --git a/notmuch-client.h b/notmuch-client.h\r
 271 > > index 517c010..e85f882 100644\r
 272 > > --- a/notmuch-client.h\r
 273 > > +++ b/notmuch-client.h\r
 274 > > @@ -30,6 +30,12 @@\r
 275 > >=20=20\r
 276 > >  #include <gmime/gmime.h>\r
 277 > >=20=20\r
 278 > > +/* GMIME_CHECK_VERSION is only available in gmime >=3D 2.5. But so are\r
 279 > > + * GMIME_MAJOR_VERSION and friends. */\r
 280 >=20\r
 281 > Hah.\r
 282 >=20\r
 283 > > +#ifdef GMIME_MAJOR_VERSION\r
 284 > > +#define GMIME_26\r
 285 > > +#endif\r
 286 > > +\r
 287 > >  #include "notmuch.h"\r
 288 > >=20=20\r
 289 > >  /* This is separate from notmuch-private.h because we're trying to\r
 290 > > @@ -69,7 +75,11 @@ typedef struct notmuch_show_format {\r
 291 > >      void (*part_start) (GMimeObject *part,\r
 292 > >                     int *part_count);\r
 293 > >      void (*part_encstatus) (int status);\r
 294 > > +#ifdef GMIME_26\r
 295 > > +    void (*part_sigstatus) (GMimeSignatureList* siglist);\r
 296 > > +#else\r
 297 > >      void (*part_sigstatus) (const GMimeSignatureValidity* validity);\r
 298 > > +#endif\r
 299 > >      void (*part_content) (GMimeObject *part);\r
 300 > >      void (*part_end) (GMimeObject *part);\r
 301 > >      const char *part_sep;\r
 302 > > @@ -83,7 +93,11 @@ typedef struct notmuch_show_params {\r
 303 > >      int entire_thread;\r
 304 > >      int raw;\r
 305 > >      int part;\r
 306 > > +#ifdef GMIME_26\r
 307 > > +    GMimeCryptoContext* cryptoctx;\r
 308 > > +#else\r
 309 > >      GMimeCipherContext* cryptoctx;\r
 310 > > +#endif\r
 311 > >      int decrypt;\r
 312 > >  } notmuch_show_params_t;\r
 313 > >=20=20\r
 314 > > @@ -286,7 +300,12 @@ typedef struct mime_node {\r
 315 > >       * signature.  May be NULL if signature verification failed.  If\r
 316 > >       * there are simply no signatures, this will be non-NULL with an\r
 317 > >       * empty signers list. */\r
 318 > > +#ifdef GMIME_26\r
 319 > > +    /* TODO: update the above comment... */\r
 320 >=20\r
 321 > Since this behaves very differently in 2.6, I think documenting it is\r
 322 > important (and being very careful about the differences).  Maybe\r
 323 >=20\r
 324 > /* The list of signatures for signed or encrypted containers.  If\r
 325 >  * there are no signatures, this will be NULL. */\r
 326 >=20\r
 327 > > +    GMimeSignatureList* sig_list;\r
 328 > > +#else\r
 329 > >      const GMimeSignatureValidity *sig_validity;\r
 330 > > +#endif\r
 331 > >=20=20\r
 332 > >      /* Internal: Context inherited from the root iterator. */\r
 333 > >      struct mime_node_context *ctx;\r
 334 > > @@ -311,8 +330,12 @@ typedef struct mime_node {\r
 335 > >   */\r
 336 > >  notmuch_status_t\r
 337 > >  mime_node_open (const void *ctx, notmuch_message_t *message,\r
 338 > > -           GMimeCipherContext *cryptoctx, notmuch_bool_t decrypt,\r
 339 > > -           mime_node_t **node_out);\r
 340 > > +#ifdef GMIME_26\r
 341 > > +           GMimeCryptoContext *cryptoctx,\r
 342 > > +#else\r
 343 > > +           GMimeCipherContext *cryptoctx,\r
 344 > > +#endif\r
 345 > > +           notmuch_bool_t decrypt, mime_node_t **node_out);\r
 346 > >=20=20\r
 347 > >  /* Return a new MIME node for the requested child part of parent.\r
 348 > >   * parent will be used as the talloc context for the returned child\r
 349 > > diff --git a/notmuch-reply.c b/notmuch-reply.c\r
 350 > > index da3acce..dc37c51 100644\r
 351 > > --- a/notmuch-reply.c\r
 352 > > +++ b/notmuch-reply.c\r
 353 > > @@ -688,15 +688,22 @@ notmuch_reply_command (void *ctx, int argc, char =\r
 354 *argv[])\r
 355 > >     reply_format_func =3D notmuch_reply_format_default;\r
 356 > >=20=20\r
 357 > >      if (decrypt) {\r
 358 > > +#ifdef GMIME_26\r
 359 > > +   /* TODO: GMimePasswordRequestFunc */\r
 360 > > +   params.cryptoctx =3D g_mime_gpg_context_new (NULL, "gpg");\r
 361 > > +#else\r
 362 > >     GMimeSession* session =3D g_object_new (g_mime_session_get_type(), NU=\r
 363 LL);\r
 364 > >     params.cryptoctx =3D g_mime_gpg_context_new (session, "gpg");\r
 365 > > +#endif\r
 366 > >     if (params.cryptoctx) {\r
 367 > >         g_mime_gpg_context_set_always_trust ((GMimeGpgContext*) params.cr=\r
 368 yptoctx, FALSE);\r
 369 > >         params.decrypt =3D TRUE;\r
 370 > >     } else {\r
 371 > >         fprintf (stderr, "Failed to construct gpg context.\n");\r
 372 > >     }\r
 373 > > +#ifndef GMIME_26\r
 374 > >     g_object_unref (session);\r
 375 > > +#endif\r
 376 > >      }\r
 377 > >=20=20\r
 378 > >      config =3D notmuch_config_open (ctx, NULL, NULL);\r
 379 > > diff --git a/notmuch-show.c b/notmuch-show.c\r
 380 > > index d14dac9..263ab72 100644\r
 381 > > --- a/notmuch-show.c\r
 382 > > +++ b/notmuch-show.c\r
 383 > > @@ -76,7 +76,11 @@ static void\r
 384 > >  format_part_encstatus_json (int status);\r
 385 > >=20=20\r
 386 > >  static void\r
 387 > > +#ifdef GMIME_26\r
 388 > > +format_part_sigstatus_json (GMimeSignatureList* siglist);\r
 389 > > +#else\r
 390 > >  format_part_sigstatus_json (const GMimeSignatureValidity* validity);\r
 391 > > +#endif\r
 392 > >=20=20\r
 393 > >  static void\r
 394 > >  format_part_content_json (GMimeObject *part);\r
 395 > > @@ -486,6 +490,21 @@ show_text_part_content (GMimeObject *part, GMimeSt=\r
 396 ream *stream_out)\r
 397 > >     g_object_unref(stream_filter);\r
 398 > >  }\r
 399 > >=20=20\r
 400 > > +#ifdef GMIME_26\r
 401 > > +static const char*\r
 402 > > +signature_status_to_string (GMimeSignatureStatus x)\r
 403 > > +{\r
 404 > > +    switch (x) {\r
 405 > > +    case GMIME_SIGNATURE_STATUS_GOOD:\r
 406 > > +   return "good";\r
 407 > > +    case GMIME_SIGNATURE_STATUS_BAD:\r
 408 > > +   return "bad";\r
 409 > > +    case GMIME_SIGNATURE_STATUS_ERROR:\r
 410 > > +   return "error";\r
 411 > > +    }\r
 412 > > +    return "unknown";\r
 413 > > +}\r
 414 > > +#else\r
 415 > >  static const char*\r
 416 > >  signer_status_to_string (GMimeSignerStatus x)\r
 417 > >  {\r
 418 > > @@ -501,6 +520,7 @@ signer_status_to_string (GMimeSignerStatus x)\r
 419 > >      }\r
 420 > >      return "unknown";\r
 421 > >  }\r
 422 > > +#endif\r
 423 > >=20=20\r
 424 > >  static void\r
 425 > >  format_part_start_text (GMimeObject *part, int *part_count)\r
 426 > > @@ -592,6 +612,75 @@ format_part_encstatus_json (int status)\r
 427 > >      printf ("}]");\r
 428 > >  }\r
 429 > >=20=20\r
 430 > > +#ifdef GMIME_26\r
 431 > > +static void\r
 432 > > +format_part_sigstatus_json (GMimeSignatureList *siglist)\r
 433 > > +{\r
 434 > > +    printf (", \"sigstatus\": [");\r
 435 > > +\r
 436 > > +    if (!siglist) {\r
 437 > > +   printf ("]");\r
 438 > > +   return;\r
 439 > > +     }\r
 440 > > +\r
 441 > > +    void *ctx_quote =3D talloc_new (NULL);\r
 442 > > +    int i;\r
 443 > > +    for (i =3D 0; i < g_mime_signature_list_length (siglist); ++i) {\r
 444 >=20\r
 445 > Style nit: notmuch uses i++.\r
 446 >=20\r
 447 > > +   GMimeSignature *signature =3D g_mime_signature_list_get_signature (si=\r
 448 glist, i);\r
 449 > > +\r
 450 > > +   if (i > 0)\r
 451 > > +       printf (", ");\r
 452 > > +\r
 453 > > +   printf ("{");\r
 454 > > +\r
 455 > > +   /* status */\r
 456 > > +   GMimeSignatureStatus status =3D g_mime_signature_get_status (signatur=\r
 457 e);\r
 458 > > +   printf ("\"status\": %s",\r
 459 > > +           json_quote_str (ctx_quote,\r
 460 > > +                           signature_status_to_string (status)));\r
 461 > > +\r
 462 > > +   GMimeCertificate *certificate =3D g_mime_signature_get_certificate (s=\r
 463 ignature);\r
 464 > > +   if (status =3D=3D GMIME_SIGNATURE_STATUS_GOOD)\r
 465 > > +   {\r
 466 >=20\r
 467 > Style nit: break after brace.\r
 468 >=20\r
 469 > (Presumably this is copied from the existing\r
 470 > format_part_sigstatus_json, but since it's technically new code\r
 471 > there's no reason not to fix these up.)\r
 472 >=20\r
 473 > > +       if (certificate)\r
 474 > > +           printf (", \"fingerprint\": %s", json_quote_str (ctx_quote, g_mime_c=\r
 475 ertificate_get_fingerprint (certificate)));\r
 476 > > +       /* these dates are seconds since the epoch; should we\r
 477 > > +        * provide a more human-readable format string? */\r
 478 > > +       time_t created =3D g_mime_signature_get_created (signature);\r
 479 > > +       if (created !=3D -1)\r
 480 > > +           printf (", \"created\": %d", (int) created);\r
 481 > > +       time_t expires =3D g_mime_signature_get_expires (signature);\r
 482 > > +       if (expires > 0)\r
 483 > > +           printf (", \"expires\": %d", (int) expires);\r
 484 >=20\r
 485 > Is it intentional that the two above checks are different?  I would\r
 486 > think the second should be expires !=3D -1.\r
 487 \r
 488 Yes, it should check for =E2=80=9Cexpires !=3D -1=E2=80=9D. Also, wouldn't =\r
 489 it be needed to\r
 490 cast do =E2=80=9Cexpires !=3D (time_t) -1=E2=80=9D to avoid compiler warnin=\r
 491 gs? (time_t may\r
 492 be different from int.)\r
 493 =20\r
 494 > > +       /* output user id only if validity is FULL or ULTIMATE. */\r
 495 > > +       /* note that gmime is using the term "trust" here, which\r
 496 > > +        * is WRONG.  It's actually user id "validity". */\r
 497 > > +       if (certificate)\r
 498 > > +       {\r
 499 >=20\r
 500 > Break after brace.\r
 501 >=20\r
 502 > > +           const char *name =3D g_mime_certificate_get_name (certificate);\r
 503 > > +           GMimeCertificateTrust trust =3D g_mime_certificate_get_trust (certif=\r
 504 icate);\r
 505 > > +           if (name && (trust =3D=3D GMIME_CERTIFICATE_TRUST_FULLY || trust =3D=\r
 506 =3D GMIME_CERTIFICATE_TRUST_ULTIMATE))\r
 507 > > +               printf (", \"userid\": %s", json_quote_str (ctx_quote, name));\r
 508 > > +       }\r
 509 > > +   } else if (certificate) {\r
 510 > > +       const char *key_id =3D g_mime_certificate_get_key_id (certificate=\r
 511 );\r
 512 > > +       if (key_id)\r
 513 > > +           printf (", \"keyid\": %s", json_quote_str (ctx_quote, key_id));\r
 514 > > +   }\r
 515 > > +\r
 516 > > +   GMimeSignatureError errors =3D g_mime_signature_get_errors (signature=\r
 517 );\r
 518 > > +   if (errors !=3D GMIME_SIGNATURE_ERROR_NONE) {\r
 519 > > +       printf (", \"errors\": %x", errors);\r
 520 >=20\r
 521 > This should be %d (I would say 0x%x, but JSON doesn't support hex\r
 522 > literals).  I see this bug came from the original\r
 523 > format_part_sigstatus_json.  Maybe there should be a quick patch\r
 524 > before this one that fixes the source of the bug?\r
 525 >=20\r
 526 > > +   }\r
 527 > > +\r
 528 > > +   printf ("}");\r
 529 > > +     }\r
 530 > > +\r
 531 > > +    printf ("]");\r
 532 > > +\r
 533 > > +    talloc_free (ctx_quote);\r
 534 > > +}\r
 535 > > +#else\r
 536 > >  static void\r
 537 > >  format_part_sigstatus_json (const GMimeSignatureValidity* validity)\r
 538 > >  {\r
 539 > > @@ -652,6 +741,7 @@ format_part_sigstatus_json (const GMimeSignatureVal=\r
 540 idity* validity)\r
 541 > >=20=20\r
 542 > >      talloc_free (ctx_quote);\r
 543 > >  }\r
 544 > > +#endif\r
 545 > >=20=20\r
 546 > >  static void\r
 547 > >  format_part_content_json (GMimeObject *part)\r
 548 > > @@ -990,13 +1080,20 @@ notmuch_show_command (void *ctx, unused (int arg=\r
 549 c), unused (char *argv[]))\r
 550 > >     } else if ((STRNCMP_LITERAL (argv[i], "--verify") =3D=3D 0) ||\r
 551 > >                (STRNCMP_LITERAL (argv[i], "--decrypt") =3D=3D 0)) {\r
 552 > >         if (params.cryptoctx =3D=3D NULL) {\r
 553 > > +#ifdef GMIME_26\r
 554 > > +           /* TODO: GMimePasswordRequestFunc */\r
 555 > > +           if (NULL =3D=3D (params.cryptoctx =3D g_mime_gpg_context_new(NULL, "=\r
 556 gpg")))\r
 557 > > +#else\r
 558 > >             GMimeSession* session =3D g_object_new(g_mime_session_get_type(), NU=\r
 559 LL);\r
 560 > >             if (NULL =3D=3D (params.cryptoctx =3D g_mime_gpg_context_new(session=\r
 561 , "gpg")))\r
 562 > > +#endif\r
 563 > >                 fprintf (stderr, "Failed to construct gpg context.\n");\r
 564 > >             else\r
 565 > >                 g_mime_gpg_context_set_always_trust((GMimeGpgContext*)params.cry=\r
 566 ptoctx, FALSE);\r
 567 > > +#ifndef GMIME_26\r
 568 > >             g_object_unref (session);\r
 569 > >             session =3D NULL;\r
 570 > > +#endif\r
 571 > >         }\r
 572 > >         if (STRNCMP_LITERAL (argv[i], "--decrypt") =3D=3D 0)\r
 573 > >             params.decrypt =3D 1;\r
 574 > > diff --git a/show-message.c b/show-message.c\r
 575 > > index 8768889..65269fd 100644\r
 576 > > --- a/show-message.c\r
 577 > > +++ b/show-message.c\r
 578 > > @@ -48,7 +48,11 @@ show_message_part (mime_node_t *node,\r
 579 > >     format->part_encstatus (node->decrypt_success);\r
 580 > >=20=20\r
 581 > >      if (node->verify_attempted && format->part_sigstatus)\r
 582 > > +#ifdef GMIME_26\r
 583 > > +   format->part_sigstatus (node->sig_list);\r
 584 > > +#else\r
 585 > >     format->part_sigstatus (node->sig_validity);\r
 586 > > +#endif\r
 587 > >=20=20\r
 588 > >      format->part_content (part);\r
 589 > >=20=20\r
 590 \r
 591 =2D-=20\r
 592 Adrian Perez <aperez@igalia.com> - Sent from my toaster\r
 593 Igalia - Free Software Engineering\r
 594 \r
 595 --=-=-=\r
 596 Content-Type: application/pgp-signature\r
 597 \r
 598 -----BEGIN PGP SIGNATURE-----\r
 599 Version: GnuPG v1.4.11 (GNU/Linux)\r
 600 \r
 601 iEYEARECAAYFAk8VUe4ACgkQkcVZ2+TJEjtcwgCaAjJvz03iNmXNkcc/g76IsYcB\r
 602 fNoAn0SWJIRc+5XxE/xkXo7Cxfno3crv\r
 603 =RWWF\r
 604 -----END PGP SIGNATURE-----\r
 605 --=-=-=--\r