[PATCH 2/4] Introduce a generic tree-like abstraction for MIME traversal.

[Jameson Graef Rollins]

On Sat, 10 Dec 2011 03:25:48 +0400, Dmitry Kurochkin <dmitry.kurochkin at gmail.com> wrote:
> +    notmuch_bool_t decrypt_success;
> 
> Perhaps s/decrypt_success/is_decrypted/ for consistency with
> is_encrypted?

This difference doesn't seem so bad to me, since the is_ variables point
to states of the original message, where as decrypt_success reflects the
processing of the message only.

> +           out->is_encrypted = TRUE;
> +           out->is_signed = TRUE;
> 
> These are set only if we do decryption/verification.  But their
> names and comments imply that they should reflect properties of
> the part and be set independently from whether we are actually
> doing decryption or verification.

I don't think this directly affects anything at the moment, but this is
a good point.  I can imagine that it might be good to maintain that
distinction down the line so it's probably worth being consistent here.

> +           /* For some reason the GMimeSignatureValidity returned
> +            * here is not a const (inconsistent with that
> +            * returned by
> +            * g_mime_multipart_encrypted_get_signature_validity,
> +            * and therefore needs to be properly disposed of.
> +            * Hopefully the API will become more consistent. */
> 
> Ouch.  In gmime 2.6 this API has changed, but looks like the
> issue is still there.  Is there a bug for it?  If yes, we should
> add a reference to the comment.  Otherwise, we should file the
> bug and then add a reference to the comment :)

Don't blame any of this stuff on Austin.  This is left over from the
original crypto processing patches.  I know dkg was in touch with the
gmime maintainers on this issue, but I'm not sure if a bug was filed.

> Both decryption and verification use cryptoctx.  But decryption
> is done iff decrypt is true (without checking if cryptoctx is
> set) and verification is done iff cryptoctx is set (without any
> special flag).  Why is it asymmetric?  Do we need to check if
> cryptoctx is set for decryption?

We probably should.  We can probably fix this in followup patches, since
 Austin is just working off of what dkg and I put in there originally.

jamie.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: not available
URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20111210/8967893e/attachment.pgp>