Re: [Evolution-hackers] https://bugzilla.gnome.org/show_bug.cgi?id=697632

[samarjit Adhikari <samarjit adhikari gmail com>]

Thanks Milan for the review.

>Good catch, the e_mail_reader_get_folder() doesn't return reffed
>folder, but the mail-to-task still unrefs the folder. I guess it's
>some artifact from the time when the function was introduced. I see
>few more places in the mail-to-task.c which does the unref, please
>update your patch and attach it to bugzilla.

That's the confusion I have. So will the good fix be a g_object_ref inside "e_mail_reader_get_folder" on folder object? or is it ok to delete the unref. In that case I need to delete all unref  consistently in mail-to-task.

With regards,

Samarjit

On Mon, Apr 15, 2013 at 1:55 PM, Milan Crha <mcrha redhat com> wrote:

On Mon, 2013-04-15 at 12:20 +0530, samarjit Adhikari wrote:
> Probably I have managed to cook a fix for bug 697632. Code review is
> required to confirm the fix.
>
> I am not sure how to create patch. I took a diff and attaching here.

        Hi,
thanks for the work on this. If it's related to a bug, then attach patch
there, not to the mailing list. From a quick review:
a) the '//' is a C++ comment, you get a compiler warning on it; if the
   unref is not needed, then just delete it.
b) how did you get the diff? I suppose it's not from a git clone (with
   'git diff'), because the 'git diff' shows also a function name
   in which you did the change, while your diff doesn't do that.
c) Good catch, the e_mail_reader_get_folder() doesn't return reffed
   folder, but the mail-to-task still unrefs the folder. I guess it's
   some artifact from the time when the function was introduced. I see
   few more places in the mail-to-task.c which does the unref, please
   update your patch and attach it to bugzilla.
        Thanks and bye,
        Milan

_______________________________________________
evolution-hackers mailing list
evolution-hackers gnome org
To change your list options or unsubscribe, visit ...
https://mail.gnome.org/mailman/listinfo/evolution-hackers