From: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
To: David Bremner <david@tethera.net>, Notmuch Mail <notmuch@notmuchmail.org>
Subject: Re: [PATCH v3 03/16] make shared crypto code behave library-like
In-Reply-To: <87twlhuw8f.fsf@maritornes.cs.unb.ca>
References: <1454272801-23623-1-git-send-email-dkg@fifthhorseman.net>
 <1454272801-23623-4-git-send-email-dkg@fifthhorseman.net>
 <87twlhuw8f.fsf@maritornes.cs.unb.ca>
User-Agent: Notmuch/0.21+72~gd8c4f1c (http://notmuchmail.org) Emacs/24.5.1
 (x86_64-pc-linux-gnu)
Date: Wed, 10 Feb 2016 11:18:18 -0500
Message-ID: <87d1s4o7xx.fsf@alice.fifthhorseman.net>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: list

On Tue 2016-02-09 21:37:04 -0500, David Bremner wrote:
> Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:
>> -static GMimeCryptoContext*
>> -create_gpg_context (_notmuch_crypto_t *crypto)
>> +static notmuch_status_t
>> +get_gpg_context (_notmuch_crypto_t *crypto, GMimeCryptoContext **ctx)
>
> I was a littled puzzled by this renaming. I guess it's fine, it just
> creates a bit more diff noise.

the renaming from create_ to get_ is meaningful -- this function won't
actually create a new gpg context if one already exists.

>>  /* Create a PKCS7 context (GMime 2.6) */
>> -static notmuch_crypto_context_t *
>> -create_pkcs7_context (notmuch_crypto_t *crypto)
>> +static notmuch_status_t 
>> +create_pkcs7_context (_notmuch_crypto_t *crypto, GMimeCryptoContext **ctx)
>
> Especially since you only renamed the gpg version. Or did I miss
> something?

ah, you're right here, this should be fixed.

>> +notmuch_status_t
>> +_notmuch_crypto_get_gmime_ctx_for_protocol (_notmuch_crypto_t *crypto,
>> +					    const char *protocol,
>> +					    GMimeCryptoContext **ctx)
>>  {
>> -    GMimeCryptoContext *cryptoctx = NULL;
>> -    size_t i;
>> -
>> -    if (! protocol) {
>> -	fprintf (stderr, "Cryptographic protocol is empty.\n");
>> -	return cryptoctx;
>> -    }
>> +    if (! protocol)
>> +	return NOTMUCH_STATUS_MALFORMED_CRYPTO_PROTOCOL;
>>  
>>      /* As per RFC 1847 section 2.1: "the [protocol] value token is
>>       * comprised of the type and sub-type tokens of the Content-Type".
>> @@ -112,15 +112,12 @@ _notmuch_crypto_get_gmime_context (_notmuch_crypto_t *crypto, const char *protoc
>>       * parameter names as defined in this document are
>>       * case-insensitive."  Thus, we use strcasecmp for the protocol.
>>       */
>> -    for (i = 0; i < ARRAY_SIZE (protocols); i++) {
>> +    for (size_t i = 0; i < ARRAY_SIZE (protocols); i++) {
>
> I forget whether we are using C99 for loop variable declarations.  I
> like them myself. If someone figures out the answer, maybe they can
> update devel/STYLE.

is what i did above a C99 loop variable declaration?  If so, i also like
them.  I've left it in for now :)

       --dkg