From: Jameson Graef Rollins <jrollins@finestructure.net>
To: Dmitry Kurochkin <dmitry.kurochkin@gmail.com>,
	Austin Clements <amdragon@MIT.EDU>, notmuch@notmuchmail.org
Subject: Re: [PATCH 2/4] Introduce a generic tree-like abstraction for MIME
	traversal.
In-Reply-To: <87k46572f7.fsf@gmail.com>
References: <1323027100-10307-1-git-send-email-amdragon@mit.edu>
	<1323460468-4030-1-git-send-email-amdragon@mit.edu>
	<1323460468-4030-3-git-send-email-amdragon@mit.edu>
	<87k46572f7.fsf@gmail.com>
User-Agent: Notmuch/0.10.2+74~g994a706 (http://notmuchmail.org) Emacs/23.3.1
	(x86_64-pc-linux-gnu)
Date: Sat, 10 Dec 2011 13:17:02 -0800
Message-ID: <87mxb05dpt.fsf@servo.finestructure.net>
MIME-Version: 1.0
Content-Type: multipart/signed; boundary="=-=-=";
	micalg=pgp-sha256; protocol="application/pgp-signature"
Precedence: list

--=-=-=
Content-Transfer-Encoding: quoted-printable

On Sat, 10 Dec 2011 03:25:48 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmai=
l.com> wrote:
> +    notmuch_bool_t decrypt_success;
>=20
> 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 =3D TRUE;
> +           out->is_signed =3D TRUE;
>=20
> 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. */
>=20
> 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.

--=-=-=
Content-Type: application/pgp-signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)

iQIcBAEBCAAGBQJO48xOAAoJEO00zqvie6q8mhYP/3No5YtwAyqFUw9HZxe6zKJR
5KeicO8ZhMMbs/hH91jxmI3xPdN4psA79aLbaVIfNbd+GLJm2domRYUlzF+92E4t
XnXX3nNHc5mHW+TGz8t1DAhSqOcZcHbMgypWYbyimuwjQhr3fDKkXVD/iOnZPTAq
bDtvnTrat/Nw9nOza5gX9FeZsyNhQQgV9S3GadV6TROHcTBCnmEdx7b3ljaeMwBf
s/c7ng0U8OeoyAEVXR+cDjXGpwFmCfywwUEk7mUVlVLLh4hQjhsO4ljdG04MDsCX
2hRGLIpL1091rsfHH5qVElDcHmszk2yljedZ942hJP4IxRVUmAooELomBdFQYzlV
Kk5ebPKAdWUr9Bu2vw/U2oMdcpVUEDhvdr+0cUYbcTn5fE9jdmUxXBroixB42H5o
N9VdDj3IBP5gbr/sWdFhQiyMpOZZvy9/VbG6kGkN8GxTOBjQZnTpzRF7RayWODNv
JTyKbkbksJTe7RATElJNmCthEZ5aXY9w4LPWYzuGeHZ9coUlnXJMjl8h3D7QzLur
BgHXfRdZ7P4gqDIC0HTOKGhgL34S0WTpRU3Na3mEN9Wu7vnv6hjZVDTDFjZ4jK/a
MVjH5LxA3075byUwX4BIhohQR2QZDicQc+axtPu8dR79GNHdlBt/XdplOpHhTuPu
wVjUUXfhtBOR0Hcym7uw
=UHUD
-----END PGP SIGNATURE-----
--=-=-=--