[PATCH v2 2/2] Add pseudo-compatibility with gmime 2.6

2012-01-20 Thread Thomas Jost 
On Wed, 18 Jan 2012 12:35:34 -0500, Austin Clements  wrote:
> Quoth Tomi Ollila on Jan 18 at 10:15 am:
> > On Tue, 17 Jan 2012 17:25:46 -0500, Austin Clements  
> > wrote:
> > > Quoth Thomas Jost on Jan 17 at 11:50 am:
> > > >  
> > > > +#ifdef GMIME_26
> > > > +/* sig_list may be created in both above cases, so we need to
> > > > + * cleanly handle it here. */
> > > > +if (node->sig_list) {
> > > > +   GMimeSignatureList **proxy =
> > > > +   talloc (node, GMimeSignatureList *);
> > > 
> > > This doesn't need to be split into two lines.
> > > 
> > > > +   *proxy = node->sig_list;
> > > > +   talloc_set_destructor (proxy, _signature_list_free);
> > > > +}
> > > > +#else
> > > >  if (node->verify_attempted && !node->sig_validity)
> > > > fprintf (stderr, "Failed to verify signed part: %s\n",
> > > >  (err ? err->message : "no error explanation given"));
> > > > +#endif
> > > 
> > > I'd rather see the above as a separate #ifdef GMIME_26 and #ifndef
> > > GMIME_26, since they aren't logical alternates of each other.
> > 
> > That reminds me that it should then be like GMIME_ATLEAST_26, so
> > that this might be useful when GMIME > 2.6 is available...
> 
> Hopefully before GMIME 2.8 comes out, we'll be able to remove all of
> the GMIME 2.4 compatibility.  But GMIME_ATLEAST_26 would be fine, too.

Heh, maybe things will change again in 2.8 and "ATLEAST_26" will become
"ONLY_26"... But changed to GMIME_ATLEAST_26 anyway, thanks for the
suggestion :)

-- 
Thomas/Schnouki
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 489 bytes
Desc: not available
URL:

[PATCH v2 2/2] Add pseudo-compatibility with gmime 2.6

2012-01-20 Thread Thomas Jost 
Thanks for this review Austin. New version coming soon.

On Tue, 17 Jan 2012 17:25:46 -0500, Austin Clements  wrote:
> Quoth Thomas Jost on Jan 17 at 11:50 am:
> > There are lots of API changes in gmime 2.6 crypto handling. By adding
> > preprocessor directives, it is however possible to add gmime 2.6 
> > compatibility
> > while preserving compatibility with gmime 2.4 too.
> > 
> > This is mostly based on id:"8762i8hrb9.fsf at bookbinder.fernseed.info".
> > 
> > This was tested against both gmime 2.6.4 and 2.4.31. With gmime 2.4.31, the
> > crypto tests all work fine (as expected). With gmime 2.6.4, one crypto test
> > fails (signature verification with signer key unavailable) but this will be 
> > hard
> > to fix since the new API does not report the reason why a signature 
> > verification
> > fails (other than the human-readable error message).
> > ---
> >  mime-node.c  |   56 ++--
> >  notmuch-client.h |   28 +++-
> >  notmuch-reply.c  |7 
> >  notmuch-show.c   |   95 
> > ++
> >  show-message.c   |4 ++
> >  5 files changed, 185 insertions(+), 5 deletions(-)
> > 
> > diff --git a/mime-node.c b/mime-node.c
> > index d26bb44..e575e1c 100644
> > --- a/mime-node.c
> > +++ b/mime-node.c
> > @@ -33,7 +33,11 @@ typedef struct mime_node_context {
> >  GMimeMessage *mime_message;
> >  
> >  /* Context provided by the caller. */
> > +#ifdef GMIME_26
> > +GMimeCryptoContext *cryptoctx;
> > +#else
> >  GMimeCipherContext *cryptoctx;
> > +#endif
> >  notmuch_bool_t decrypt;
> >  } mime_node_context_t;
> >  
> > @@ -57,8 +61,12 @@ _mime_node_context_free (mime_node_context_t *res)
> >  
> >  notmuch_status_t
> >  mime_node_open (const void *ctx, notmuch_message_t *message,
> > -   GMimeCipherContext *cryptoctx, notmuch_bool_t decrypt,
> > -   mime_node_t **root_out)
> > +#ifdef GMIME_26
> > +   GMimeCryptoContext *cryptoctx,
> > +#else
> > +   GMimeCipherContext *cryptoctx,
> > +#endif
> > +   notmuch_bool_t decrypt, mime_node_t **root_out)
> >  {
> >  const char *filename = notmuch_message_get_filename (message);
> >  mime_node_context_t *mctx;
> > @@ -112,12 +120,21 @@ DONE:
> >  return status;
> >  }
> >  
> > +#ifdef GMIME_26
> > +static int
> > +_signature_list_free (GMimeSignatureList **proxy)
> > +{
> > +g_object_unref (*proxy);
> > +return 0;
> > +}
> > +#else
> >  static int
> >  _signature_validity_free (GMimeSignatureValidity **proxy)
> >  {
> >  g_mime_signature_validity_free (*proxy);
> >  return 0;
> >  }
> > +#endif
> >  
> >  static mime_node_t *
> >  _mime_node_create (const mime_node_t *parent, GMimeObject *part)
> > @@ -165,11 +182,22 @@ _mime_node_create (const mime_node_t *parent, 
> > GMimeObject *part)
> > GMimeMultipartEncrypted *encrypteddata =
> > GMIME_MULTIPART_ENCRYPTED (part);
> > node->decrypt_attempted = TRUE;
> > +#ifdef GMIME_26
> > +   GMimeDecryptResult *decrypt_result = NULL;
> 
> Reading through the GMime code, it looks like we do have to unref
> decrypt_result.  I think this is easy, though.  Right after you call
> g_mime_decrypt_result_get_signatures below, do:
> 
>   g_object_ref (node->sig_list);
>   g_object_unref (decrypt_result);

Added, thanks!

> 
> > +   node->decrypted_child = g_mime_multipart_encrypted_decrypt
> > +   (encrypteddata, node->ctx->cryptoctx, _result, );
> > +#else
> > node->decrypted_child = g_mime_multipart_encrypted_decrypt
> > (encrypteddata, node->ctx->cryptoctx, );
> > +#endif
> > if (node->decrypted_child) {
> > -   node->decrypt_success = node->verify_attempted = TRUE;
> > +   node->decrypt_success = node->verify_attempted =TRUE;
> 
> Whitespace.  (There should be no diff on the above line)

Oops, my bad.

> 
> > +#ifdef GMIME_26
> > +   /* This may be NULL if the part is not signed. */
> > +   node->sig_list = g_mime_decrypt_result_get_signatures 
> > (decrypt_result);
> > +#else
> > node->sig_validity = 
> > g_mime_multipart_encrypted_get_signature_validity (encrypteddata);
> > +#endif
> > } else {
> > fprintf (stderr, "Failed to decrypt part: %s\n",
> >  (err ? err->message : "no error explanation given"));
> > @@ -182,6 +210,16 @@ _mime_node_create (const mime_node_t *parent, 
> > GMimeObject *part)
> >  "(must be exactly 2)\n",
> >  node->nchildren);
> > } else {
> > +#ifdef GMIME_26
> > +   GMimeSignatureList *sig_list = g_mime_multipart_signed_verify
> > +   (GMIME_MULTIPART_SIGNED (part), node->ctx->cryptoctx, );
> > +   node->verify_attempted = TRUE;
> > +   node->sig_list = sig_list;
> 
> Just a nit.  This could be
>   node->sig_list = g_mime_multipart_signed_verify ( ... )
> To me, the local variable just makes this code more verbose

Re: [PATCH v2 2/2] Add pseudo-compatibility with gmime 2.6

2012-01-19 Thread Thomas Jost 
Thanks for this review Austin. New version coming soon.

On Tue, 17 Jan 2012 17:25:46 -0500, Austin Clements amdra...@mit.edu wrote:
 Quoth Thomas Jost on Jan 17 at 11:50 am:
  There are lots of API changes in gmime 2.6 crypto handling. By adding
  preprocessor directives, it is however possible to add gmime 2.6 
  compatibility
  while preserving compatibility with gmime 2.4 too.
  
  This is mostly based on id:8762i8hrb9@bookbinder.fernseed.info.
  
  This was tested against both gmime 2.6.4 and 2.4.31. With gmime 2.4.31, the
  crypto tests all work fine (as expected). With gmime 2.6.4, one crypto test
  fails (signature verification with signer key unavailable) but this will be 
  hard
  to fix since the new API does not report the reason why a signature 
  verification
  fails (other than the human-readable error message).
  ---
   mime-node.c  |   56 ++--
   notmuch-client.h |   28 +++-
   notmuch-reply.c  |7 
   notmuch-show.c   |   95 
  ++
   show-message.c   |4 ++
   5 files changed, 185 insertions(+), 5 deletions(-)
  
  diff --git a/mime-node.c b/mime-node.c
  index d26bb44..e575e1c 100644
  --- a/mime-node.c
  +++ b/mime-node.c
  @@ -33,7 +33,11 @@ typedef struct mime_node_context {
   GMimeMessage *mime_message;
   
   /* Context provided by the caller. */
  +#ifdef GMIME_26
  +GMimeCryptoContext *cryptoctx;
  +#else
   GMimeCipherContext *cryptoctx;
  +#endif
   notmuch_bool_t decrypt;
   } mime_node_context_t;
   
  @@ -57,8 +61,12 @@ _mime_node_context_free (mime_node_context_t *res)
   
   notmuch_status_t
   mime_node_open (const void *ctx, notmuch_message_t *message,
  -   GMimeCipherContext *cryptoctx, notmuch_bool_t decrypt,
  -   mime_node_t **root_out)
  +#ifdef GMIME_26
  +   GMimeCryptoContext *cryptoctx,
  +#else
  +   GMimeCipherContext *cryptoctx,
  +#endif
  +   notmuch_bool_t decrypt, mime_node_t **root_out)
   {
   const char *filename = notmuch_message_get_filename (message);
   mime_node_context_t *mctx;
  @@ -112,12 +120,21 @@ DONE:
   return status;
   }
   
  +#ifdef GMIME_26
  +static int
  +_signature_list_free (GMimeSignatureList **proxy)
  +{
  +g_object_unref (*proxy);
  +return 0;
  +}
  +#else
   static int
   _signature_validity_free (GMimeSignatureValidity **proxy)
   {
   g_mime_signature_validity_free (*proxy);
   return 0;
   }
  +#endif
   
   static mime_node_t *
   _mime_node_create (const mime_node_t *parent, GMimeObject *part)
  @@ -165,11 +182,22 @@ _mime_node_create (const mime_node_t *parent, 
  GMimeObject *part)
  GMimeMultipartEncrypted *encrypteddata =
  GMIME_MULTIPART_ENCRYPTED (part);
  node-decrypt_attempted = TRUE;
  +#ifdef GMIME_26
  +   GMimeDecryptResult *decrypt_result = NULL;
 
 Reading through the GMime code, it looks like we do have to unref
 decrypt_result.  I think this is easy, though.  Right after you call
 g_mime_decrypt_result_get_signatures below, do:
 
   g_object_ref (node-sig_list);
   g_object_unref (decrypt_result);

Added, thanks!

 
  +   node-decrypted_child = g_mime_multipart_encrypted_decrypt
  +   (encrypteddata, node-ctx-cryptoctx, decrypt_result, err);
  +#else
  node-decrypted_child = g_mime_multipart_encrypted_decrypt
  (encrypteddata, node-ctx-cryptoctx, err);
  +#endif
  if (node-decrypted_child) {
  -   node-decrypt_success = node-verify_attempted = TRUE;
  +   node-decrypt_success = node-verify_attempted =TRUE;
 
 Whitespace.  (There should be no diff on the above line)

Oops, my bad.

 
  +#ifdef GMIME_26
  +   /* This may be NULL if the part is not signed. */
  +   node-sig_list = g_mime_decrypt_result_get_signatures 
  (decrypt_result);
  +#else
  node-sig_validity = 
  g_mime_multipart_encrypted_get_signature_validity (encrypteddata);
  +#endif
  } else {
  fprintf (stderr, Failed to decrypt part: %s\n,
   (err ? err-message : no error explanation given));
  @@ -182,6 +210,16 @@ _mime_node_create (const mime_node_t *parent, 
  GMimeObject *part)
   (must be exactly 2)\n,
   node-nchildren);
  } else {
  +#ifdef GMIME_26
  +   GMimeSignatureList *sig_list = g_mime_multipart_signed_verify
  +   (GMIME_MULTIPART_SIGNED (part), node-ctx-cryptoctx, err);
  +   node-verify_attempted = TRUE;
  +   node-sig_list = sig_list;
 
 Just a nit.  This could be
   node-sig_list = g_mime_multipart_signed_verify ( ... )
 To me, the local variable just makes this code more verbose without
 adding anything.  Up to you.

Yep, the local variable is useless. Removed it.

 
  +
  +   if (!sig_list)
  +   fprintf (stderr, Failed to verify signed part: %s\n,
  +(err ? err-message : no error

Re: [PATCH v2 2/2] Add pseudo-compatibility with gmime 2.6

2012-01-19 Thread Thomas Jost 
On Wed, 18 Jan 2012 20:00:12 +0200, Tomi Ollila tomi.oll...@iki.fi wrote:
 On Wed, 18 Jan 2012 12:19:45 -0500, Tom Prince tom.pri...@ualberta.net 
 wrote:
  
  How did you test against multiple versions? Using different machines? If
  there was a way for configure (or something to pick the version, I would
  setup the buildbot to test against both, so we don't regress either.
 
 I currently compile notmuch on Fedora 15 so that I have 
 
 LIBRARY_PATH=/my/own/path/to/x86_64-linux/lib
 and
 CPATH=/my/own/path/to/x86_64-linux/include
 
 and gmime 2.4 development files are located in these
 directories. I'll be hiding gmime 2.4 stuff from these
 soon in order to build against 2.6 stuff.
 
 Tomi

For testing gmime 2.4 and 2.6, I just uninstalled 2.6 and reinstalled
2.4 (kept the binary package on purpose -- not a problem since notmuch
is the only package using gmime on my system).

When hacking the gmime git in order to fix
https://bugzilla.gnome.org/show_bug.cgi?id=668085, I set some
environment variables before calling ./configure and building notmuch:

  LDFLAGS=-Wl,-rpath,/home/schnouki/dev/gmime/gmime/.libs ./configure 
--prefix=/usr --sysconfdir=/etc
  make
  ldd ./notmuch

Regards,

-- 
Thomas/Schnouki


pgpybiDupPPyc.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH v2 2/2] Add pseudo-compatibility with gmime 2.6

2012-01-18 Thread Tomi Ollila 
On Wed, 18 Jan 2012 12:19:45 -0500, Tom Prince  
wrote:
> 
> How did you test against multiple versions? Using different machines? If
> there was a way for configure (or something to pick the version, I would
> setup the buildbot to test against both, so we don't regress either.

I currently compile notmuch on Fedora 15 so that I have 

LIBRARY_PATH=/my/own/path/to/x86_64-linux/lib
and
CPATH=/my/own/path/to/x86_64-linux/include

and gmime 2.4 development files are located in these
directories. I'll be hiding gmime 2.4 stuff from these
soon in order to build against 2.6 stuff.

Tomi

[PATCH v2 2/2] Add pseudo-compatibility with gmime 2.6

2012-01-18 Thread Tom Prince 
On Tue, 17 Jan 2012 11:50:53 +0100, Thomas Jost  
wrote:
> This was tested against both gmime 2.6.4 and 2.4.31. With gmime 2.4.31, the
> crypto tests all work fine (as expected). With gmime 2.6.4, one crypto test
> fails (signature verification with signer key unavailable) but this will be 
> hard
> to fix since the new API does not report the reason why a signature 
> verification
> fails (other than the human-readable error message).

How did you test against multiple versions? Using different machines? If
there was a way for configure (or something to pick the version, I would
setup the buildbot to test against both, so we don't regress either.

  Tom

[PATCH v2 2/2] Add pseudo-compatibility with gmime 2.6

2012-01-18 Thread Tomi Ollila 
On Tue, 17 Jan 2012 17:25:46 -0500, Austin Clements  wrote:
> Quoth Thomas Jost on Jan 17 at 11:50 am:
> >  
> > +#ifdef GMIME_26
> > +/* sig_list may be created in both above cases, so we need to
> > + * cleanly handle it here. */
> > +if (node->sig_list) {
> > +   GMimeSignatureList **proxy =
> > +   talloc (node, GMimeSignatureList *);
> 
> This doesn't need to be split into two lines.
> 
> > +   *proxy = node->sig_list;
> > +   talloc_set_destructor (proxy, _signature_list_free);
> > +}
> > +#else
> >  if (node->verify_attempted && !node->sig_validity)
> > fprintf (stderr, "Failed to verify signed part: %s\n",
> >  (err ? err->message : "no error explanation given"));
> > +#endif
> 
> I'd rather see the above as a separate #ifdef GMIME_26 and #ifndef
> GMIME_26, since they aren't logical alternates of each other.

That reminds me that it should then be like GMIME_ATLEAST_26, so
that this might be useful when GMIME > 2.6 is available...

Tomi

Re: [PATCH v2 2/2] Add pseudo-compatibility with gmime 2.6

2012-01-18 Thread Tomi Ollila 
On Tue, 17 Jan 2012 17:25:46 -0500, Austin Clements amdra...@mit.edu wrote:
 Quoth Thomas Jost on Jan 17 at 11:50 am:
   
  +#ifdef GMIME_26
  +/* sig_list may be created in both above cases, so we need to
  + * cleanly handle it here. */
  +if (node-sig_list) {
  +   GMimeSignatureList **proxy =
  +   talloc (node, GMimeSignatureList *);
 
 This doesn't need to be split into two lines.
 
  +   *proxy = node-sig_list;
  +   talloc_set_destructor (proxy, _signature_list_free);
  +}
  +#else
   if (node-verify_attempted  !node-sig_validity)
  fprintf (stderr, Failed to verify signed part: %s\n,
   (err ? err-message : no error explanation given));
  +#endif
 
 I'd rather see the above as a separate #ifdef GMIME_26 and #ifndef
 GMIME_26, since they aren't logical alternates of each other.

That reminds me that it should then be like GMIME_ATLEAST_26, so
that this might be useful when GMIME  2.6 is available...

Tomi
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v2 2/2] Add pseudo-compatibility with gmime 2.6

2012-01-18 Thread Tom Prince 
On Tue, 17 Jan 2012 11:50:53 +0100, Thomas Jost schno...@schnouki.net wrote:
 This was tested against both gmime 2.6.4 and 2.4.31. With gmime 2.4.31, the
 crypto tests all work fine (as expected). With gmime 2.6.4, one crypto test
 fails (signature verification with signer key unavailable) but this will be 
 hard
 to fix since the new API does not report the reason why a signature 
 verification
 fails (other than the human-readable error message).

How did you test against multiple versions? Using different machines? If
there was a way for configure (or something to pick the version, I would
setup the buildbot to test against both, so we don't regress either.

  Tom
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v2 2/2] Add pseudo-compatibility with gmime 2.6

2012-01-18 Thread Tomi Ollila 
On Wed, 18 Jan 2012 12:19:45 -0500, Tom Prince tom.pri...@ualberta.net wrote:
 
 How did you test against multiple versions? Using different machines? If
 there was a way for configure (or something to pick the version, I would
 setup the buildbot to test against both, so we don't regress either.

I currently compile notmuch on Fedora 15 so that I have 

LIBRARY_PATH=/my/own/path/to/x86_64-linux/lib
and
CPATH=/my/own/path/to/x86_64-linux/include

and gmime 2.4 development files are located in these
directories. I'll be hiding gmime 2.4 stuff from these
soon in order to build against 2.6 stuff.

Tomi
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH v2 2/2] Add pseudo-compatibility with gmime 2.6

2012-01-17 Thread Austin Clements 
Quoth Thomas Jost on Jan 17 at 11:50 am:
> There are lots of API changes in gmime 2.6 crypto handling. By adding
> preprocessor directives, it is however possible to add gmime 2.6 compatibility
> while preserving compatibility with gmime 2.4 too.
> 
> This is mostly based on id:"8762i8hrb9.fsf at bookbinder.fernseed.info".
> 
> This was tested against both gmime 2.6.4 and 2.4.31. With gmime 2.4.31, the
> crypto tests all work fine (as expected). With gmime 2.6.4, one crypto test
> fails (signature verification with signer key unavailable) but this will be 
> hard
> to fix since the new API does not report the reason why a signature 
> verification
> fails (other than the human-readable error message).
> ---
>  mime-node.c  |   56 ++--
>  notmuch-client.h |   28 +++-
>  notmuch-reply.c  |7 
>  notmuch-show.c   |   95 
> ++
>  show-message.c   |4 ++
>  5 files changed, 185 insertions(+), 5 deletions(-)
> 
> diff --git a/mime-node.c b/mime-node.c
> index d26bb44..e575e1c 100644
> --- a/mime-node.c
> +++ b/mime-node.c
> @@ -33,7 +33,11 @@ typedef struct mime_node_context {
>  GMimeMessage *mime_message;
>  
>  /* Context provided by the caller. */
> +#ifdef GMIME_26
> +GMimeCryptoContext *cryptoctx;
> +#else
>  GMimeCipherContext *cryptoctx;
> +#endif
>  notmuch_bool_t decrypt;
>  } mime_node_context_t;
>  
> @@ -57,8 +61,12 @@ _mime_node_context_free (mime_node_context_t *res)
>  
>  notmuch_status_t
>  mime_node_open (const void *ctx, notmuch_message_t *message,
> - GMimeCipherContext *cryptoctx, notmuch_bool_t decrypt,
> - mime_node_t **root_out)
> +#ifdef GMIME_26
> + GMimeCryptoContext *cryptoctx,
> +#else
> + GMimeCipherContext *cryptoctx,
> +#endif
> + notmuch_bool_t decrypt, mime_node_t **root_out)
>  {
>  const char *filename = notmuch_message_get_filename (message);
>  mime_node_context_t *mctx;
> @@ -112,12 +120,21 @@ DONE:
>  return status;
>  }
>  
> +#ifdef GMIME_26
> +static int
> +_signature_list_free (GMimeSignatureList **proxy)
> +{
> +g_object_unref (*proxy);
> +return 0;
> +}
> +#else
>  static int
>  _signature_validity_free (GMimeSignatureValidity **proxy)
>  {
>  g_mime_signature_validity_free (*proxy);
>  return 0;
>  }
> +#endif
>  
>  static mime_node_t *
>  _mime_node_create (const mime_node_t *parent, GMimeObject *part)
> @@ -165,11 +182,22 @@ _mime_node_create (const mime_node_t *parent, 
> GMimeObject *part)
>   GMimeMultipartEncrypted *encrypteddata =
>   GMIME_MULTIPART_ENCRYPTED (part);
>   node->decrypt_attempted = TRUE;
> +#ifdef GMIME_26
> + GMimeDecryptResult *decrypt_result = NULL;

Reading through the GMime code, it looks like we do have to unref
decrypt_result.  I think this is easy, though.  Right after you call
g_mime_decrypt_result_get_signatures below, do:

  g_object_ref (node->sig_list);
  g_object_unref (decrypt_result);

> + node->decrypted_child = g_mime_multipart_encrypted_decrypt
> + (encrypteddata, node->ctx->cryptoctx, _result, );
> +#else
>   node->decrypted_child = g_mime_multipart_encrypted_decrypt
>   (encrypteddata, node->ctx->cryptoctx, );
> +#endif
>   if (node->decrypted_child) {
> - node->decrypt_success = node->verify_attempted = TRUE;
> + node->decrypt_success = node->verify_attempted =TRUE;

Whitespace.  (There should be no diff on the above line)

> +#ifdef GMIME_26
> + /* This may be NULL if the part is not signed. */
> + node->sig_list = g_mime_decrypt_result_get_signatures 
> (decrypt_result);
> +#else
>   node->sig_validity = 
> g_mime_multipart_encrypted_get_signature_validity (encrypteddata);
> +#endif
>   } else {
>   fprintf (stderr, "Failed to decrypt part: %s\n",
>(err ? err->message : "no error explanation given"));
> @@ -182,6 +210,16 @@ _mime_node_create (const mime_node_t *parent, 
> GMimeObject *part)
>"(must be exactly 2)\n",
>node->nchildren);
>   } else {
> +#ifdef GMIME_26
> + GMimeSignatureList *sig_list = g_mime_multipart_signed_verify
> + (GMIME_MULTIPART_SIGNED (part), node->ctx->cryptoctx, );
> + node->verify_attempted = TRUE;
> + node->sig_list = sig_list;

Just a nit.  This could be
  node->sig_list = g_mime_multipart_signed_verify ( ... )
To me, the local variable just makes this code more verbose without
adding anything.  Up to you.

> +
> + if (!sig_list)
> + fprintf (stderr, "Failed to verify signed part: %s\n",
> +  (err ? err->message : "no error explanation given"));
> +#else
>   /* For some reason the GMimeSignatureValidity returned
>* here is not a const (inconsistent with

[PATCH v2 2/2] Add pseudo-compatibility with gmime 2.6

2012-01-17 Thread Tomi Ollila 
On Tue, 17 Jan 2012 11:50:53 +0100, Thomas Jost  
wrote:
> There are lots of API changes in gmime 2.6 crypto handling. By adding
> preprocessor directives, it is however possible to add gmime 2.6 compatibility
> while preserving compatibility with gmime 2.4 too.
> 
> This is mostly based on id:"8762i8hrb9.fsf at bookbinder.fernseed.info".
> 
> This was tested against both gmime 2.6.4 and 2.4.31. With gmime 2.4.31, the
> crypto tests all work fine (as expected). With gmime 2.6.4, one crypto test
> fails (signature verification with signer key unavailable) but this will be 
> hard
> to fix since the new API does not report the reason why a signature 
> verification
> fails (other than the human-readable error message).
> ---

LGTM. Some whitespace things but most of those were there already;
I'd have one uncrustify round to be applied to the source and after
that be more strict about those...

actually gmime 2.4 has GMIME_CHECK_VERSION defined as
g_mime_check_version (major, minor, micro) so the comment about it
is not entirely accurate (it is unusable in our context, though)


Tomi

[PATCH v2 2/2] Add pseudo-compatibility with gmime 2.6

2012-01-17 Thread Thomas Jost 
On Tue, 17 Jan 2012 14:48:34 +0200, Tomi Ollila  wrote:
> On Tue, 17 Jan 2012 11:50:53 +0100, Thomas Jost  
> wrote:
> > There are lots of API changes in gmime 2.6 crypto handling. By adding
> > preprocessor directives, it is however possible to add gmime 2.6 
> > compatibility
> > while preserving compatibility with gmime 2.4 too.
> > 
> > This is mostly based on id:"8762i8hrb9.fsf at bookbinder.fernseed.info".
> > 
> > This was tested against both gmime 2.6.4 and 2.4.31. With gmime 2.4.31, the
> > crypto tests all work fine (as expected). With gmime 2.6.4, one crypto test
> > fails (signature verification with signer key unavailable) but this will be 
> > hard
> > to fix since the new API does not report the reason why a signature 
> > verification
> > fails (other than the human-readable error message).
> > ---
> 
> LGTM. Some whitespace things but most of those were there already;
> I'd have one uncrustify round to be applied to the source and after
> that be more strict about those...

Thanks! I'll fix the whitespace issues in these patches when the source
in Git is uncrustified then.

> actually gmime 2.4 has GMIME_CHECK_VERSION defined as
> g_mime_check_version (major, minor, micro) so the comment about it
> is not entirely accurate (it is unusable in our context, though)

Oops, yes. What I really meant is that GMIME_MAJOR_VERSION is not
available as a preprocessor constant in 2.4, and GMIME_CHECK_VERSION is
unusable in our context since it just calls a runtime function.

By the way, how do you guys feel about setting GMIME_26 in
notmuch-client.h? Is that good enough, or should it be done in configure
so that gcc is called with "-DGMIME_26"?

I filed a bug about gmime 2.6 incorrect handling of signatures with
missing public keys: https://bugzilla.gnome.org/show_bug.cgi?id=668085.
A patch is attached there, feel free to test and comment.

(Arch Linux users: I made a little PKGBUILD that includes this patch if
you want to build your own gmime 2.6.4:
http://fichiers.schnouki.net/tmp/gmime-2.6.4-1.src.tar.gz)

Best regards,

-- 
Thomas/Schnouki
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 489 bytes
Desc: not available
URL:

[PATCH v2 2/2] Add pseudo-compatibility with gmime 2.6

2012-01-17 Thread Thomas Jost 
There are lots of API changes in gmime 2.6 crypto handling. By adding
preprocessor directives, it is however possible to add gmime 2.6 compatibility
while preserving compatibility with gmime 2.4 too.

This is mostly based on id:"8762i8hrb9.fsf at bookbinder.fernseed.info".

This was tested against both gmime 2.6.4 and 2.4.31. With gmime 2.4.31, the
crypto tests all work fine (as expected). With gmime 2.6.4, one crypto test
fails (signature verification with signer key unavailable) but this will be hard
to fix since the new API does not report the reason why a signature verification
fails (other than the human-readable error message).
---
 mime-node.c  |   56 ++--
 notmuch-client.h |   28 +++-
 notmuch-reply.c  |7 
 notmuch-show.c   |   95 ++
 show-message.c   |4 ++
 5 files changed, 185 insertions(+), 5 deletions(-)

diff --git a/mime-node.c b/mime-node.c
index d26bb44..e575e1c 100644
--- a/mime-node.c
+++ b/mime-node.c
@@ -33,7 +33,11 @@ typedef struct mime_node_context {
 GMimeMessage *mime_message;

 /* Context provided by the caller. */
+#ifdef GMIME_26
+GMimeCryptoContext *cryptoctx;
+#else
 GMimeCipherContext *cryptoctx;
+#endif
 notmuch_bool_t decrypt;
 } mime_node_context_t;

@@ -57,8 +61,12 @@ _mime_node_context_free (mime_node_context_t *res)

 notmuch_status_t
 mime_node_open (const void *ctx, notmuch_message_t *message,
-   GMimeCipherContext *cryptoctx, notmuch_bool_t decrypt,
-   mime_node_t **root_out)
+#ifdef GMIME_26
+   GMimeCryptoContext *cryptoctx,
+#else
+   GMimeCipherContext *cryptoctx,
+#endif
+   notmuch_bool_t decrypt, mime_node_t **root_out)
 {
 const char *filename = notmuch_message_get_filename (message);
 mime_node_context_t *mctx;
@@ -112,12 +120,21 @@ DONE:
 return status;
 }

+#ifdef GMIME_26
+static int
+_signature_list_free (GMimeSignatureList **proxy)
+{
+g_object_unref (*proxy);
+return 0;
+}
+#else
 static int
 _signature_validity_free (GMimeSignatureValidity **proxy)
 {
 g_mime_signature_validity_free (*proxy);
 return 0;
 }
+#endif

 static mime_node_t *
 _mime_node_create (const mime_node_t *parent, GMimeObject *part)
@@ -165,11 +182,22 @@ _mime_node_create (const mime_node_t *parent, GMimeObject 
*part)
GMimeMultipartEncrypted *encrypteddata =
GMIME_MULTIPART_ENCRYPTED (part);
node->decrypt_attempted = TRUE;
+#ifdef GMIME_26
+   GMimeDecryptResult *decrypt_result = NULL;
+   node->decrypted_child = g_mime_multipart_encrypted_decrypt
+   (encrypteddata, node->ctx->cryptoctx, _result, );
+#else
node->decrypted_child = g_mime_multipart_encrypted_decrypt
(encrypteddata, node->ctx->cryptoctx, );
+#endif
if (node->decrypted_child) {
-   node->decrypt_success = node->verify_attempted = TRUE;
+   node->decrypt_success = node->verify_attempted =TRUE;
+#ifdef GMIME_26
+   /* This may be NULL if the part is not signed. */
+   node->sig_list = g_mime_decrypt_result_get_signatures 
(decrypt_result);
+#else
node->sig_validity = 
g_mime_multipart_encrypted_get_signature_validity (encrypteddata);
+#endif
} else {
fprintf (stderr, "Failed to decrypt part: %s\n",
 (err ? err->message : "no error explanation given"));
@@ -182,6 +210,16 @@ _mime_node_create (const mime_node_t *parent, GMimeObject 
*part)
 "(must be exactly 2)\n",
 node->nchildren);
} else {
+#ifdef GMIME_26
+   GMimeSignatureList *sig_list = g_mime_multipart_signed_verify
+   (GMIME_MULTIPART_SIGNED (part), node->ctx->cryptoctx, );
+   node->verify_attempted = TRUE;
+   node->sig_list = sig_list;
+
+   if (!sig_list)
+   fprintf (stderr, "Failed to verify signed part: %s\n",
+(err ? err->message : "no error explanation given"));
+#else
/* For some reason the GMimeSignatureValidity returned
 * here is not a const (inconsistent with that
 * returned by
@@ -200,12 +238,24 @@ _mime_node_create (const mime_node_t *parent, GMimeObject 
*part)
*proxy = sig_validity;
talloc_set_destructor (proxy, _signature_validity_free);
}
+#endif
}
 }

+#ifdef GMIME_26
+/* sig_list may be created in both above cases, so we need to
+ * cleanly handle it here. */
+if (node->sig_list) {
+   GMimeSignatureList **proxy =
+   talloc (node, GMimeSignatureList *);
+   *proxy = node->sig_list;
+   talloc_set_destructor (proxy, _signature_list_free);
+}
+#else
 if (node->verify_attempted && !node->sig_validity)
fprintf (stderr, "Failed to verify signed

[PATCH v2 2/2] Add pseudo-compatibility with gmime 2.6

2012-01-17 Thread Thomas Jost 
There are lots of API changes in gmime 2.6 crypto handling. By adding
preprocessor directives, it is however possible to add gmime 2.6 compatibility
while preserving compatibility with gmime 2.4 too.

This is mostly based on id:8762i8hrb9@bookbinder.fernseed.info.

This was tested against both gmime 2.6.4 and 2.4.31. With gmime 2.4.31, the
crypto tests all work fine (as expected). With gmime 2.6.4, one crypto test
fails (signature verification with signer key unavailable) but this will be hard
to fix since the new API does not report the reason why a signature verification
fails (other than the human-readable error message).
---
 mime-node.c  |   56 ++--
 notmuch-client.h |   28 +++-
 notmuch-reply.c  |7 
 notmuch-show.c   |   95 ++
 show-message.c   |4 ++
 5 files changed, 185 insertions(+), 5 deletions(-)

diff --git a/mime-node.c b/mime-node.c
index d26bb44..e575e1c 100644
--- a/mime-node.c
+++ b/mime-node.c
@@ -33,7 +33,11 @@ typedef struct mime_node_context {
 GMimeMessage *mime_message;
 
 /* Context provided by the caller. */
+#ifdef GMIME_26
+GMimeCryptoContext *cryptoctx;
+#else
 GMimeCipherContext *cryptoctx;
+#endif
 notmuch_bool_t decrypt;
 } mime_node_context_t;
 
@@ -57,8 +61,12 @@ _mime_node_context_free (mime_node_context_t *res)
 
 notmuch_status_t
 mime_node_open (const void *ctx, notmuch_message_t *message,
-   GMimeCipherContext *cryptoctx, notmuch_bool_t decrypt,
-   mime_node_t **root_out)
+#ifdef GMIME_26
+   GMimeCryptoContext *cryptoctx,
+#else
+   GMimeCipherContext *cryptoctx,
+#endif
+   notmuch_bool_t decrypt, mime_node_t **root_out)
 {
 const char *filename = notmuch_message_get_filename (message);
 mime_node_context_t *mctx;
@@ -112,12 +120,21 @@ DONE:
 return status;
 }
 
+#ifdef GMIME_26
+static int
+_signature_list_free (GMimeSignatureList **proxy)
+{
+g_object_unref (*proxy);
+return 0;
+}
+#else
 static int
 _signature_validity_free (GMimeSignatureValidity **proxy)
 {
 g_mime_signature_validity_free (*proxy);
 return 0;
 }
+#endif
 
 static mime_node_t *
 _mime_node_create (const mime_node_t *parent, GMimeObject *part)
@@ -165,11 +182,22 @@ _mime_node_create (const mime_node_t *parent, GMimeObject 
*part)
GMimeMultipartEncrypted *encrypteddata =
GMIME_MULTIPART_ENCRYPTED (part);
node-decrypt_attempted = TRUE;
+#ifdef GMIME_26
+   GMimeDecryptResult *decrypt_result = NULL;
+   node-decrypted_child = g_mime_multipart_encrypted_decrypt
+   (encrypteddata, node-ctx-cryptoctx, decrypt_result, err);
+#else
node-decrypted_child = g_mime_multipart_encrypted_decrypt
(encrypteddata, node-ctx-cryptoctx, err);
+#endif
if (node-decrypted_child) {
-   node-decrypt_success = node-verify_attempted = TRUE;
+   node-decrypt_success = node-verify_attempted =TRUE;
+#ifdef GMIME_26
+   /* This may be NULL if the part is not signed. */
+   node-sig_list = g_mime_decrypt_result_get_signatures 
(decrypt_result);
+#else
node-sig_validity = 
g_mime_multipart_encrypted_get_signature_validity (encrypteddata);
+#endif
} else {
fprintf (stderr, Failed to decrypt part: %s\n,
 (err ? err-message : no error explanation given));
@@ -182,6 +210,16 @@ _mime_node_create (const mime_node_t *parent, GMimeObject 
*part)
 (must be exactly 2)\n,
 node-nchildren);
} else {
+#ifdef GMIME_26
+   GMimeSignatureList *sig_list = g_mime_multipart_signed_verify
+   (GMIME_MULTIPART_SIGNED (part), node-ctx-cryptoctx, err);
+   node-verify_attempted = TRUE;
+   node-sig_list = sig_list;
+
+   if (!sig_list)
+   fprintf (stderr, Failed to verify signed part: %s\n,
+(err ? err-message : no error explanation given));
+#else
/* For some reason the GMimeSignatureValidity returned
 * here is not a const (inconsistent with that
 * returned by
@@ -200,12 +238,24 @@ _mime_node_create (const mime_node_t *parent, GMimeObject 
*part)
*proxy = sig_validity;
talloc_set_destructor (proxy, _signature_validity_free);
}
+#endif
}
 }
 
+#ifdef GMIME_26
+/* sig_list may be created in both above cases, so we need to
+ * cleanly handle it here. */
+if (node-sig_list) {
+   GMimeSignatureList **proxy =
+   talloc (node, GMimeSignatureList *);
+   *proxy = node-sig_list;
+   talloc_set_destructor (proxy, _signature_list_free);
+}
+#else
 if (node-verify_attempted  !node-sig_validity)
fprintf (stderr, Failed to verify signed part: %s\n,

Re: [PATCH v2 2/2] Add pseudo-compatibility with gmime 2.6

2012-01-17 Thread Tomi Ollila 
On Tue, 17 Jan 2012 11:50:53 +0100, Thomas Jost schno...@schnouki.net wrote:
 There are lots of API changes in gmime 2.6 crypto handling. By adding
 preprocessor directives, it is however possible to add gmime 2.6 compatibility
 while preserving compatibility with gmime 2.4 too.
 
 This is mostly based on id:8762i8hrb9@bookbinder.fernseed.info.
 
 This was tested against both gmime 2.6.4 and 2.4.31. With gmime 2.4.31, the
 crypto tests all work fine (as expected). With gmime 2.6.4, one crypto test
 fails (signature verification with signer key unavailable) but this will be 
 hard
 to fix since the new API does not report the reason why a signature 
 verification
 fails (other than the human-readable error message).
 ---

LGTM. Some whitespace things but most of those were there already;
I'd have one uncrustify round to be applied to the source and after
that be more strict about those...

actually gmime 2.4 has GMIME_CHECK_VERSION defined as
g_mime_check_version (major, minor, micro) so the comment about it
is not entirely accurate (it is unusable in our context, though)


Tomi
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v2 2/2] Add pseudo-compatibility with gmime 2.6

2012-01-17 Thread Thomas Jost 
On Tue, 17 Jan 2012 14:48:34 +0200, Tomi Ollila tomi.oll...@iki.fi wrote:
 On Tue, 17 Jan 2012 11:50:53 +0100, Thomas Jost schno...@schnouki.net wrote:
  There are lots of API changes in gmime 2.6 crypto handling. By adding
  preprocessor directives, it is however possible to add gmime 2.6 
  compatibility
  while preserving compatibility with gmime 2.4 too.
  
  This is mostly based on id:8762i8hrb9@bookbinder.fernseed.info.
  
  This was tested against both gmime 2.6.4 and 2.4.31. With gmime 2.4.31, the
  crypto tests all work fine (as expected). With gmime 2.6.4, one crypto test
  fails (signature verification with signer key unavailable) but this will be 
  hard
  to fix since the new API does not report the reason why a signature 
  verification
  fails (other than the human-readable error message).
  ---
 
 LGTM. Some whitespace things but most of those were there already;
 I'd have one uncrustify round to be applied to the source and after
 that be more strict about those...

Thanks! I'll fix the whitespace issues in these patches when the source
in Git is uncrustified then.

 actually gmime 2.4 has GMIME_CHECK_VERSION defined as
 g_mime_check_version (major, minor, micro) so the comment about it
 is not entirely accurate (it is unusable in our context, though)

Oops, yes. What I really meant is that GMIME_MAJOR_VERSION is not
available as a preprocessor constant in 2.4, and GMIME_CHECK_VERSION is
unusable in our context since it just calls a runtime function.

By the way, how do you guys feel about setting GMIME_26 in
notmuch-client.h? Is that good enough, or should it be done in configure
so that gcc is called with -DGMIME_26?

I filed a bug about gmime 2.6 incorrect handling of signatures with
missing public keys: https://bugzilla.gnome.org/show_bug.cgi?id=668085.
A patch is attached there, feel free to test and comment.

(Arch Linux users: I made a little PKGBUILD that includes this patch if
you want to build your own gmime 2.6.4:
http://fichiers.schnouki.net/tmp/gmime-2.6.4-1.src.tar.gz)

Best regards,

-- 
Thomas/Schnouki


pgp3y7I0N927m.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v2 2/2] Add pseudo-compatibility with gmime 2.6

2012-01-17 Thread Austin Clements 
Quoth Thomas Jost on Jan 17 at 11:50 am:
 There are lots of API changes in gmime 2.6 crypto handling. By adding
 preprocessor directives, it is however possible to add gmime 2.6 compatibility
 while preserving compatibility with gmime 2.4 too.
 
 This is mostly based on id:8762i8hrb9@bookbinder.fernseed.info.
 
 This was tested against both gmime 2.6.4 and 2.4.31. With gmime 2.4.31, the
 crypto tests all work fine (as expected). With gmime 2.6.4, one crypto test
 fails (signature verification with signer key unavailable) but this will be 
 hard
 to fix since the new API does not report the reason why a signature 
 verification
 fails (other than the human-readable error message).
 ---
  mime-node.c  |   56 ++--
  notmuch-client.h |   28 +++-
  notmuch-reply.c  |7 
  notmuch-show.c   |   95 
 ++
  show-message.c   |4 ++
  5 files changed, 185 insertions(+), 5 deletions(-)
 
 diff --git a/mime-node.c b/mime-node.c
 index d26bb44..e575e1c 100644
 --- a/mime-node.c
 +++ b/mime-node.c
 @@ -33,7 +33,11 @@ typedef struct mime_node_context {
  GMimeMessage *mime_message;
  
  /* Context provided by the caller. */
 +#ifdef GMIME_26
 +GMimeCryptoContext *cryptoctx;
 +#else
  GMimeCipherContext *cryptoctx;
 +#endif
  notmuch_bool_t decrypt;
  } mime_node_context_t;
  
 @@ -57,8 +61,12 @@ _mime_node_context_free (mime_node_context_t *res)
  
  notmuch_status_t
  mime_node_open (const void *ctx, notmuch_message_t *message,
 - GMimeCipherContext *cryptoctx, notmuch_bool_t decrypt,
 - mime_node_t **root_out)
 +#ifdef GMIME_26
 + GMimeCryptoContext *cryptoctx,
 +#else
 + GMimeCipherContext *cryptoctx,
 +#endif
 + notmuch_bool_t decrypt, mime_node_t **root_out)
  {
  const char *filename = notmuch_message_get_filename (message);
  mime_node_context_t *mctx;
 @@ -112,12 +120,21 @@ DONE:
  return status;
  }
  
 +#ifdef GMIME_26
 +static int
 +_signature_list_free (GMimeSignatureList **proxy)
 +{
 +g_object_unref (*proxy);
 +return 0;
 +}
 +#else
  static int
  _signature_validity_free (GMimeSignatureValidity **proxy)
  {
  g_mime_signature_validity_free (*proxy);
  return 0;
  }
 +#endif
  
  static mime_node_t *
  _mime_node_create (const mime_node_t *parent, GMimeObject *part)
 @@ -165,11 +182,22 @@ _mime_node_create (const mime_node_t *parent, 
 GMimeObject *part)
   GMimeMultipartEncrypted *encrypteddata =
   GMIME_MULTIPART_ENCRYPTED (part);
   node-decrypt_attempted = TRUE;
 +#ifdef GMIME_26
 + GMimeDecryptResult *decrypt_result = NULL;

Reading through the GMime code, it looks like we do have to unref
decrypt_result.  I think this is easy, though.  Right after you call
g_mime_decrypt_result_get_signatures below, do:

  g_object_ref (node-sig_list);
  g_object_unref (decrypt_result);

 + node-decrypted_child = g_mime_multipart_encrypted_decrypt
 + (encrypteddata, node-ctx-cryptoctx, decrypt_result, err);
 +#else
   node-decrypted_child = g_mime_multipart_encrypted_decrypt
   (encrypteddata, node-ctx-cryptoctx, err);
 +#endif
   if (node-decrypted_child) {
 - node-decrypt_success = node-verify_attempted = TRUE;
 + node-decrypt_success = node-verify_attempted =TRUE;

Whitespace.  (There should be no diff on the above line)

 +#ifdef GMIME_26
 + /* This may be NULL if the part is not signed. */
 + node-sig_list = g_mime_decrypt_result_get_signatures 
 (decrypt_result);
 +#else
   node-sig_validity = 
 g_mime_multipart_encrypted_get_signature_validity (encrypteddata);
 +#endif
   } else {
   fprintf (stderr, Failed to decrypt part: %s\n,
(err ? err-message : no error explanation given));
 @@ -182,6 +210,16 @@ _mime_node_create (const mime_node_t *parent, 
 GMimeObject *part)
(must be exactly 2)\n,
node-nchildren);
   } else {
 +#ifdef GMIME_26
 + GMimeSignatureList *sig_list = g_mime_multipart_signed_verify
 + (GMIME_MULTIPART_SIGNED (part), node-ctx-cryptoctx, err);
 + node-verify_attempted = TRUE;
 + node-sig_list = sig_list;

Just a nit.  This could be
  node-sig_list = g_mime_multipart_signed_verify ( ... )
To me, the local variable just makes this code more verbose without
adding anything.  Up to you.

 +
 + if (!sig_list)
 + fprintf (stderr, Failed to verify signed part: %s\n,
 +  (err ? err-message : no error explanation given));
 +#else
   /* For some reason the GMimeSignatureValidity returned
* here is not a const (inconsistent with that
* returned by
 @@ -200,12 +238,24 @@ _mime_node_create (const mime_node_t *parent, 
 GMimeObject *part)
   *proxy =