Re: [PATCH v3 02/16] Move crypto.c into libutil

Subject: Re: [PATCH v3 02/16] Move crypto.c into libutil

Date: Wed, 10 Feb 2016 09:34:22 -0500

To: David Bremner, Notmuch Mail

Cc:

From: Daniel Kahn Gillmor


On Tue 2016-02-09 21:21:01 -0500, David Bremner wrote:
> Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:
>
>> This prepares us for using the crypto object in both the library and
>> the client.
>>
>> i've prefixed notmuch_crypto with _ to indicate that while this can be
>> built into the library when needed, it's not something to be exported
>> or used externally.
>
> [...] snip
>
>> diff --git a/mime-node.c b/mime-node.c
>> index e96e663..a8f5670 100644
>> --- a/mime-node.c
>> +++ b/mime-node.c
>> @@ -33,7 +33,7 @@ typedef struct mime_node_context {
>>      GMimeMessage *mime_message;
>>  
>>      /* Context provided by the caller. */
>> -    notmuch_crypto_t *crypto;
>> +    _notmuch_crypto_t *crypto;
>>  } mime_node_context_t;
>
> is this maybe search and replace gone wild? or did you mean
> to rename the type as well as the functions?

this is a type in the util library (which is shared between libnotmuch
and the cli, but not exposed by libnotmuch).  As such, i deliberately
renamed the type to appear more explicitly "private".  I do not think we
want to expose it to the library API, in part because it "contaminates"
libnotmuch with the gmime API (because it explicitly contemplates the
use of GMimeCryptoContext, see below).

>>  static void
>>  node_verify (mime_node_t *node, GMimeObject *part,
>> -	     notmuch_crypto_context_t *cryptoctx)
>> +	     GMimeCryptoContext *cryptoctx)
>>  {
>
> This change of parameter type seems significant. Does it deserve a
> comment in the commit message?

I don't think this is a big change.  notmuch_crypto_context was
originally a typedef to GMimeCryptoContext, and it wasn't being exposed
by the library API.  Rather than pretend that it's something distinct, i
think it makes more explicit that this internal part of util deals with
libgmime.

        --dkg

Thread: