GNOME Bugzilla – Bug 640911
It is unclear how one should dispose of the results returned from g_mime_multipart_encrypted_decrypt () and g_mime_multipart_encrypted_get_signature_validity()
Last modified: 2011-02-08 14:10:24 UTC
g_mime_multipart_encrypted_decrypt () and g_mime_multipart_encrypted_get_signature_validity() both return pointers to structures, but the documentation doesn't make it clear whether the caller is expected to dispose of them or how to dispose of them. I suspect that the GMimeSignatureValidity returned by g_mime_multipart_encrypted_get_signature_validity() should be disposed of with g_mime_signature_validity_free(validity). I'm not sure how to properly dispose of the GMimeObject returned by g_mime_multipart_encrypted_decrypt().
You don't actually dispose of either. Both are cached on the MultipartEncrypted object after a successful call to g_mime_multipart_encrypted_decrypt(). (this is why get_validity() returns const) Looking at these functions again, it becomes clear that this bit of API is a mess and I *think* the proper solution (for 2.5.x) is to make the following changes to the decrypt() function: 1. don't cache, thus leaving it to the caller to g_object_unref() the decrypted part 2. somehow return the SignatureValidity rather than caching it, too. Any thoughts?
This workflow seems reasonable to me, but it might introduce memory leaks for people porting their code from 2.4.x to 2.5.x or 2.6.x :( Maybe do it as a renamed function (and drop the old one) with a different argument list to ensure that folks actually look at how the objects get used/disposed of? For example, maybe the caller could pass in an empty object (which they created) to recieve the decrypted contents. Why do you think that handing responsibility for disposal to the caller is the right way to go? (i'm not saying it isn't, i just curious about the rationale for either decision)
Well, when I wrote that comment I was thinking: 1. it's probably not cool to cache decrypted content (not that disposing the GObject would clear said content, but that's something else that could and perhaps should be looked at) 2. the current code is awkward and not very intuitive (some of that can be fixed with better docs, but I strive toward making the API itself intuitive if I can). I think that most users would expect that a decrypt() function would return a newly allocated MIME part containing the decrypted content rather than returning some cached object (which should not be unreffed). Developers new to mime encryption might also not be aware that a part can be signed+encrypted and having an API that returns both the decrypted part *and* the signature validity makes that more discoverable, I think. Plus I don't really like the idea of calling a second function to get a byproduct of a previous function call... it just seems kludgy. I've actually got a patch already implemented locally that does what I suggested above (since it adds a GMimeSignatureValidity ** arg to decrypt(), it will not compile old 2.4 code without some manual fixage and so that potential problem should be side-stepped). As I was implemented the patch, I noticed that the decrypted part was poked by g_mime_message_get_body(): if it comes across a multipart/encrypted that has already been decrypted, it is able to descend into that part looking for the body, else it is forced to return the encrypted part, and for this reason I haven't yet committed my patch. Well, that and I was waiting to hear any thoughts you might have ;-)
Hmm, i notice that g_mime_multipart_signed_verify() returns a non-const GMimeSignatureValidity object. How is the caller expected to dispose of that? I'm a big fan of symmetry in APIs -- is there a way to make the functions seem logically similar? http://library.gnome.org/devel/gmime/stable/gmime-GMimeMultpartSigned.html#g-mime-multipart-signed-verify
You free it using g_mime_signature_validity_free() and I agree, symmetry is good :-)
I've committed my changes (comment #3) to master now, I decided that since g_mime_message_guess_body() didn't even exist in 2.4 that to have it not descend into multipart/encrypted parts is not a great loss in functionality and that more consistency with other crypto methods was more important.