On 05/01/2013 04:59 PM, Tom Hacohen wrote:
> 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).
Super spank on this!!! So yes, it's a fix of the Eo Id commit. And what?
Spank because it is on the same commit. For that, I call you "enculeur
de mouches". Learn, it is a new french expression for you.
>
> 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.
I will check it. For the moment, EO_DEBUG is disabled (otherwise, we
will have to spank the entire world for the errors that will appear).
The most important was to push the new APIs for the release. The next
phase of the data referencing will focus on this part.
>
> 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.
Even if we don't use it, I had to initialize it. I will fix it.
> 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).
So revert. I will not push it back. And before accusing me of making
your E slow, just check WHICH COMMITS ARE AROUND MINE!!!!!!!!!!!!!!!!!
Check, not the name but the contents of each.
>
> Bottom line. Fix it. It needs fixing. Especially test stuff. Though the
> general patch looks good and clean.
As usual, the good guy sentence!
>
> --
> 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
>
------------------------------------------------------------------------------
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
