Date: Fri, 25 May 2012 10:41:36 -0400
From: Austin Clements <amdragon@MIT.EDU>
To: Jameson Graef Rollins <jrollins@finestructure.net>
Subject: Re: [PATCH v4 1/7] cli: use typedef to deal with gmime 2.4/2.6
	incompatibility
Message-ID: <20120525144136.GB11804@mit.edu>
References: <1337812843-14986-1-git-send-email-jrollins@finestructure.net>
	<1337812843-14986-2-git-send-email-jrollins@finestructure.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1337812843-14986-2-git-send-email-jrollins@finestructure.net>
User-Agent: Mutt/1.5.21 (2010-09-15)
Cc: Notmuch Mail <notmuch@notmuchmail.org>
Precedence: list

Quoth Jameson Graef Rollins on May 23 at  3:40 pm:
> gmime 2.4 defines GMimeCipherContext, while 2.6 defines
> GMimeCryptoContext.  We can use a typedef to cover this discrepancy
> and remove a bunch of #ifdefs.
> ---
>  mime-node.c      |    8 --------
>  notmuch-client.h |   10 ++--------
>  2 files changed, 2 insertions(+), 16 deletions(-)
> 
> diff --git a/mime-node.c b/mime-node.c
> index a95bdab..06fdb70 100644
> --- a/mime-node.c
> +++ b/mime-node.c
> @@ -33,11 +33,7 @@ typedef struct mime_node_context {
>      GMimeMessage *mime_message;
>  
>      /* Context provided by the caller. */
> -#ifdef GMIME_ATLEAST_26
>      GMimeCryptoContext *cryptoctx;
> -#else
> -    GMimeCipherContext *cryptoctx;
> -#endif
>      notmuch_bool_t decrypt;
>  } mime_node_context_t;
>  
> @@ -61,11 +57,7 @@ _mime_node_context_free (mime_node_context_t *res)
>  
>  notmuch_status_t
>  mime_node_open (const void *ctx, notmuch_message_t *message,
> -#ifdef GMIME_ATLEAST_26
>  		GMimeCryptoContext *cryptoctx,
> -#else
> -		GMimeCipherContext *cryptoctx,
> -#endif
>  		notmuch_bool_t decrypt, mime_node_t **root_out)
>  {
>      const char *filename = notmuch_message_get_filename (message);
> diff --git a/notmuch-client.h b/notmuch-client.h
> index 19b7f01..337409f 100644
> --- a/notmuch-client.h
> +++ b/notmuch-client.h
> @@ -36,6 +36,8 @@
>   * these to check the version number. */
>  #ifdef GMIME_MAJOR_VERSION
>  #define GMIME_ATLEAST_26
> +#else
> +typedef GMimeCipherContext GMimeCryptoContext;

I like the typedef idea, but I don't think we should overload
GMimeCryptoContext like this.  If someone is reading through the GMime
2.4 code and sees this, they're going to assume that it's a GMime
structure, go looking for it, find that it's only in 2.6 and be
baffled.  Instead, how about providing a typedef to abstract *both*
cases?  Something like

#ifdef GMIME_MAJOR_VERSION
#define GMIME_ATLEAST_26
typedef notmuch_crypto_context_t GMimeCipherContext;
#else
typedef notmuch_crypto_context_t GMimeCryptoContext;
#endif

>  #endif
>  
>  #include "notmuch.h"
> @@ -79,11 +81,7 @@ typedef struct notmuch_show_params {
>      notmuch_bool_t omit_excluded;
>      notmuch_bool_t raw;
>      int part;
> -#ifdef GMIME_ATLEAST_26
>      GMimeCryptoContext* cryptoctx;
> -#else
> -    GMimeCipherContext* cryptoctx;
> -#endif
>      notmuch_bool_t decrypt;
>  } notmuch_show_params_t;
>  
> @@ -355,11 +353,7 @@ struct mime_node {
>   */
>  notmuch_status_t
>  mime_node_open (const void *ctx, notmuch_message_t *message,
> -#ifdef GMIME_ATLEAST_26
>  		GMimeCryptoContext *cryptoctx,
> -#else
> -		GMimeCipherContext *cryptoctx,
> -#endif
>  		notmuch_bool_t decrypt, mime_node_t **node_out);
>  
>  /* Return a new MIME node for the requested child part of parent.