On 01/05/13 09:13, daniel.za...@samsung.com wrote:
> Hi all,
>
> I pushed today the data referencing feature for Eo objects in efl and
> elementary. Check the commit log for explanations on why/how we do that.
>
> From now, eo_data_get is deprecated, so please use eo_data_scope_get,
> eo_data_ref or eo_data_xref instead.
Comments:
1. Super spank, you've included unrelated changes!!!
@@ -1240,7 +1244,7 @@ eo_xunref(Eo *obj_id, const Eo *ref_obj_id)
Eo_Xref_Node *xref = NULL;
EINA_INLIST_FOREACH(obj->xrefs, xref)
{
- if (xref->ref_obj == ref_obj)
+ if (xref->ref_obj == ref_obj_id)
Fixes another issue that you've introduced (or so it seems).
2. I don't like the way you handle the data. You keep a refcount for it,
but you actually delete the data even if the reference is non-0. You
should do it like a true reference count.
You should free the data once the reference count hits 0.
You should always maintain at least one reference count (the object
referencing it's own data) so when the object is deleted that will get
decremented and the data will be freed or kept according to the
maintained reference count.
3. +_eo_data_xref_internal
You return uninitialised data if klass is NULL.
I know your code in it's current iteration won't get there, but still,
initialise it, or remove the klass!=NULL check if you think it should
never happen. At the moment it's unclear.
4. I don't understand how this even get in. You didn't add tests at all.
It's Eo, in Eo we actually test shit. I'm really tempted to just revert
it. Especially since my e got slow and annoying since your changes
(among others, haven't checked which).
Bottom line. Fix it. It needs fixing. Especially test stuff. Though the
general patch looks good and clean.
--
Tom.
------------------------------------------------------------------------------
Introducing AppDynamics Lite, a free troubleshooting tool for Java/.NET
Get 100% visibility into your production application - at no cost.
Code-level diagnostics for performance bottlenecks with <2% overhead
Download for free and get started troubleshooting in minutes.
http://p.sf.net/sfu/appdyn_d2d_ap1
_______________________________________________
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
