GNOME Bugzilla – Bug 530335
Allow Reply to selected address only
Last modified: 2013-09-13 01:04:26 UTC
Downstream bug https://bugzilla.novell.com/show_bug.cgi?id=352196 Summary: Sometimes I get an email sent to multiple addressees. If I want my reply to go to only one of those addressees, (say the person is a cc'ed addressee) I have to hit reply, remove all addresses and manually enter the correct addressee. Or, my other option is to reply all, delete the "To" addressee and move the "CC" addressee to the "To" field. Suggestion: There are options if you right-click any of the addressees in an email. Add a new option to "Reply to for this address only."
Created attachment 187271 [details] [review] Reply to any recipient of the mail by right clicking on that address & then Select "Send Reply To..." option
Review of attachment 187271 [details] [review]: Patch looks good for me.
Nope, it has memory management issues. You cannot allocate an object in e_mail_reader_reply_to_message() and do not unref it. You neither may unref it in get_recipient_cia(), as the caller will use freed memory afterwards. What is the 'cia', by the way? Could you give it less cryptic name, please?
Created attachment 189871 [details] [review] Made the necessary changes, does the patch look good now?
Thanks for the update. I guess you didn't get my complain about memory management, because the patch contains one of those above mentioned and couple other as well. a) You still call g_object_unref() in get_recipient_address(). You shouldn't do that. Use other way of indicating an error state. b) there are coding style issues with the added 'if' in e_mail_reader_reply_to_message() c) Calling g_object_unref() on a NULL object produces a runtime warning, thus never do that. d) do not unref 'address' in em_utils_reply_to_message(), the caller is responsible for its freeing (it's usually NULL, and the two whom are passing in an address pointer are unreffing it on their own, thus you introduced a double-free here) I still didn't run the patch, these are only sever issues I see with the patch while reading it.
Created attachment 189886 [details] [review] Patch modified according to your suggestions. a) I removed the function get_recipient_address() and added it directly inline within the if block itself. b) Fixed the coding style issues in if block c) The g_object_unref() is only called for a valid address value and not for a NULL object d) Removed the g_object_unref(address); statement in em_utils_reply_to_message() so that free'ing is done only once for it. Please do comment if there are any other issues to be addressed.
Thanks for a quick update. It's almost it, the only thing is in e_mail_reader_reply_to_message(), the first unref of the 'address' variable, it should set the 'addresss' to NULL too, to not unref it below in the function again. I did this little change for you, and I also changed it slightly to not panic when something goes wrong (the same function, those g_return_if_fail() and 'return' statements), but that's really another minor thing. After a little testing the patch works nicely. I also adapted it to git master, as this cannot go to stable release, because of new localized strings. Thanks for your help with this bug report.
Created commit f5794ef in evo master (3.1.3+)
Thanks a lot Milan
*** Bug 653879 has been marked as a duplicate of this bug. ***