Here is my review of the first patch elm_ctxpopup_memleak_callback.diff.
I don't have much time to review both.
1. Reduce line as possible. We don't need to block the area if the conditional
has only one
32 + EINA_LIST_FREE (sd->items, it)
33 + {
34 + elm_widget_item_free(it);
35 + }
EINA_LIST_FREE (sd->items, it)
elm_widget_item_free(it);
2. Changelog/NEWS have lack of information. People may dont understand why does
callback function fixed.
3. Don't mix the different changes in one path. This should be seprated to
memory leak and callback function to be reviewed simple and fast.
4. Please make a backport patch also.
I submitted your patch this time but please consider my reivew in next time.
Next patch i will review soon
Thank you.
------------------------------------
-Regards, Hermet-
-----Original Message-----
From: "thiep ha"<[email protected]>
To:
"[email protected]"<[email protected]>;
Cc:
Sent: 2013-01-15 (화) 15:06:45
Subject: [E-devel] [PATCH] [Elementary] Fix memory leak and callback function
in ctxpopup
Hello everyone,
There are two issues with ctxpopup:
- Memory leak: Elm_Ctxpopup_Item is used but not deleted.
- Incorrect callback function: returned object and event_info are list object
and item
(should be ctxpopup object and item).
I would like to send a patch to fix above issues.
I also provide the patch for backporting.
Please review them.
Best Regards,
Thiep
------------------------------------------------------------------------------
Master SQL Server Development, Administration, T-SQL, SSAS, SSIS, SSRS
and more. Get SQL Server skills now (including 2012) with LearnDevNow -
200+ hours of step-by-step video tutorials by Microsoft MVPs and experts.
SALE $99.99 this month only - learn more at:
http://p.sf.net/sfu/learnmore_122512
_______________________________________________
enlightenment-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
------------------------------------------------------------------------------
Master HTML5, CSS3, ASP.NET, MVC, AJAX, Knockout.js, Web API and
much more. Get web development skills now with LearnDevNow -
350+ hours of step-by-step video tutorials by Microsoft MVPs and experts.
SALE $99.99 this month only -- learn more at:
http://p.sf.net/sfu/learnmore_122812
_______________________________________________
enlightenment-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
