Hi Austin. + /* The number of children of this part. */ + int children;
Consider renaming to children_count or similar to make it clear that it is a counter and not the actual children. + notmuch_bool_t decrypt_success; Perhaps s/decrypt_success/is_decrypted/ for consistency with is_encrypted? +mime_node_child (const mime_node_t *parent, int child); + #include "command-line-arguments.h" #endif #include should go above declarations. + mime_node_t *out = talloc_zero (parent, mime_node_t); Perhaps s/out/node/? + /* this violates RFC 3156 section 4, so we won't bother with it. */ + fprintf (stderr, "Error: %d part(s) for a multipart/encrypted " + "message (should be exactly 2)\n", + out->children); Perhaps s/should be exactly/must be exactly/? That is what the RFC says. Same for signature. + 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. + /* 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 :) 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? In mime_node_child(): + GMimeMultipart *multipart = GMIME_MULTIPART (parent->part); + if (child == 1 && parent->decrypted_child) + return _mime_node_create (parent, parent->decrypted_child); Multipart is not needed here, please move it's declaration below the condition. + GMimeObject *child = g_mime_message_get_mime_part (message); The child variable eclipses the (int child) parameter. Consider using a different name for the variable (or parameter). + return _mime_node_create (parent, parent->decrypted_child); + return _mime_node_create (parent, g_mime_multipart_get_part (multipart, child)); ... + return _mime_node_create (parent, child); All returns are similar. Consider declaring a local variable for storing the part and using a single return, i.e.: GMimeObject *part; if (...) part = ...; else ... part = ...; return _mime_node_create (parent, part); Regards, Dmitry