On Fri, 22 Dec 2017 09:43:20 +0000 Andrew Williams <a...@andywilliams.me> said:
> Hi, > > I think that maybe I could have explained this a little better, let me step > back from the implementation details for a minute. > > As a user of this API all I want to know is do I have to unref a reference > that I have been handed. From this point of view we should be consistent. > My proposal was (intended) to mean that efl_add should never require the > user to unreference whereas efl_add_ref should always require this (but > only once - that may have been a bug). > > Having to know the value of the second parameter to the method to ascertain > the correct behaviour is what I think is confusing and should be > eliminated. Therefore the main purpose was so that the docs could ready > simply "this is a borrowed reference which you should not unref" vs "this > is an owned reference and you must unref when done". yes. i got that. but your proposal is no better than what is there. in fact think it's worse. unless i didn't understand it? i get your point... as i said. but your proposal doesn't sound as if it's better. what MIGHT be better is to class objects into "can have a null parent" and "must have a non-null parent". the 1st case doesn't change. from now. the 2nd case results in efl_add returning NULL (construction fails) if the parent is null. that might be an improvement. > The added complication is the race condition with the "parent" usage in > efl_add. When I pass a parent reference to efl_add then the returned handle > is something I can use. Yes this is very helpful. But this is dangerous. We > (as a framework) are providing 0 guarantee that the object will live as > long as the scope - that is entirely up to the parent, not the user and not > the framework. This is also something that I think we should address. we can't guarantee it if the parent decides some time within the scope to delete the object you added. well we can;'t unless you use efl_add_ref() then you create an object with 2 references (parent one and your scope/app one) and you will have to remember to unref at end of scope. this leads to verbose code, and having to remember to do this... which is more than likely going to lead to huge amounts of memory leaking. so which is worse? sometimes a parent decides to delete an object on you while you are still holding and using its handle, or that you always have to unref every obj you added when you exit scope? i'll take the first one any day of the week. :) > So there you go - the background behind my proposal. Hopefully if I did not > explain the details well you can now see why I felt it is important. yup. i know the background. i'm speaking of the details of the proposal. > Thanks, > Andy > > On Fri, 22 Dec 2017 at 03:25 Carsten Haitzler <ras...@rasterman.com> wrote: > > > On Thu, 21 Dec 2017 13:15:26 +0000 Andrew Williams <a...@andywilliams.me> > > said: > > > > > Hi, > > > > > > This is now well documented ( > > > https://www.enlightenment.org/develop/tutorials/c/eo-refcount.md) but > > the > > > more I use efl_add the more I feel it is confusing especially to new > > > developers. > > > > > > In the current model (if I understand it correctly) > > > 1) child = efl_add(klass, parent) means the child must NOT be > > unfeferenced > > > 2) child = efl_add(klass, NULL) means the child should be unreferenced > > > 3) child = efl_add_ref(klass, parent) means the child must be > > unreferenced > > > 4) child = efl_add_ref(klass, NULL) somehow means that the child should > > be > > > unreferenced twice > > > > #4 smells like a bug... 1 is intended. > > > > > In my opinion 1) and 4) are peculiar and so I provide a proposal to fix > > > this: > > > > > > We could change efl_add to return void. It never retains a reference. If > > > the parent is NULL then it should be automatically unref before > > returning. > > > Then efl_add_ref would be changed to mirror this and always retain > > exactly > > > 1 reference - so if parent is NULL there is no double count returned. > > > > umm... then you are saying efl_add_ref() is exactly the same as efl_add() > > today. what does this help? and the shorter efl_add now returns nothing so > > i > > can't use the return to usefully access the things i created later on like > > add > > callbacks to it, change the label of a button, delete a window, or use it > > as a > > parent for further adds which is like THE most common use case around when > > building a ui for example (create box, then create a button and pack button > > into box. i need the box to be able to do that). unless i use efl_add_ref > > which > > si the same thing as efl_add now. > > > > > Using this model if an Eo * is returned then I know I have a reference > > and > > > must later unref. > > > Any need for using the pointer in an efl_add (which is no longer > > returned) > > > would still be supported through efl_added within the constructor. > > > > if efl_add_ref returns an obj with only 1 reference, then this is wrong > > above. > > if parent is NULL, yes you'd have to unref/del. if parent is not null then > > there is still only 1 ref and it belongs to the parent object. so > > > > > What do people think about this? I put the suggestion forward to improve > > > the symettry with add and unref which is currently confusing. If my > > > assumptions above are incorrect please let me know! > > > > > > Thanks, > > > Andy > > > -- > > > http://andywilliams.me > > > http://ajwillia.ms > > > > > ------------------------------------------------------------------------------ > > > Check out the vibrant tech community on one of the world's most > > > engaging tech sites, Slashdot.org! http://sdm.link/slashdot > > > _______________________________________________ > > > enlightenment-devel mailing list > > > enlightenment-devel@lists.sourceforge.net > > > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel > > > > > > > > > -- > > ------------- Codito, ergo sum - "I code, therefore I am" -------------- > > Carsten Haitzler - ras...@rasterman.com > > > > -- > http://andywilliams.me > http://ajwillia.ms -- ------------- Codito, ergo sum - "I code, therefore I am" -------------- Carsten Haitzler - ras...@rasterman.com ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
