On Sat, 10 Dec 2011 03:25:48 +0400, Dmitry Kurochkin 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.