The example code is just a duplication within elm_code of an issue discovered. There is a elm widget in use i just wanted a quick way to reproduce the issue.
On Thu, Aug 31, 2017 at 11:01 AM, <[email protected]> wrote: > On Thu, Aug 31, 2017 at 06:44:52PM +0900, Carsten Haitzler wrote: > > On Thu, 31 Aug 2017 09:31:19 +0200 [email protected] said: > > > > > Good morning, > > > > > > On Thu, Aug 31, 2017 at 12:17:16AM -0700, Carsten Haitzler wrote: > > > > raster pushed a commit to branch master. > > > > > > > > http://git.enlightenment.org/core/efl.git/commit/?id= > 65d2dfc8925a183bf7f2709677d87edcd759e9c3 > > > > > > > > commit 65d2dfc8925a183bf7f2709677d87edcd759e9c3 > > > > Author: Carsten Haitzler (Rasterman) <[email protected]> > > > > Date: Thu Aug 31 16:12:57 2017 +0900 > > > > > > > > elm focus manager - dont leak child lists when updating focus > order > > > > > > > > this should fix T5800 > > > > > > > > @fix > > > > --- > > > > src/lib/elementary/elm_box.c | 1 + > > > > src/lib/elementary/elm_grid.c | 1 + > > > > src/lib/elementary/elm_table.c | 1 + > > > > 3 files changed, 3 insertions(+) > > > > > > > > diff --git a/src/lib/elementary/elm_box.c > b/src/lib/elementary/elm_box.c > > > > index b6a617dd2b..1c77ccd882 100644 > > > > --- a/src/lib/elementary/elm_box.c > > > > +++ b/src/lib/elementary/elm_box.c > > > > @@ -29,6 +29,7 @@ _focus_order_flush(Eo *obj, Elm_Box_Data *pd > EINA_UNUSED) > > > > Eina_List *order = evas_object_box_children_get( > wpd->resize_obj); > > > > > > > > efl_ui_focus_manager_calc_update_order(wpd->focus.manager, obj, > order); > > > > + eina_list_free(order); > > > > > > Noep, you are passing the ownership of the list to the focus manager, > > > you should NOT use the list after that anymore, esp. not freeing it, > > > same for the other usages ... :) > > > > yeah... i figured that out right after. it fixed the leak... then led to > more > > badness randomly in other cases. the issue is that failed method calls > dont > > free allocated params (also it was missing an own() in the eo file too > and > > other things). > > > > basically this will leak until eo can clean/free up the list passed in > on a > > failed call (and eo file is fixed to say it owns and the code matches > this too > > and frees the list inside the call all the time). > > > > so for now it duplicates until eo can/does do this. > > I know your manner of writing emails, so i thought twice about replying > to this. > > First of all lets break down what issues we had: > > - A missing own() in the .eo file, yep my misstake simple commit, simple > fix > > - A missing free for the case a node was not known to the manager, which > is basically the insertion of { eina_list_free(order); return; } which > is not alot of things > > - eo is not freeing parameters that where defined with own(...) > > Number 1 & 2 are my misstakes and can be fixed very simple. > > Number 3 was caused by the sample code from T5800, which just creates > an evas (NOT and efl.ui.win) and then creates a elm.widget. On the one > hand if you just spend that second looking into your terminal you would > have seen that there is a error messages indicating that this box there > should REALLY not be there... or there should be a efl.ui.win. So we have > the first fuck up here that does not really need fixing in > efl.ui.focus.manager, but rather in your calling code. > > Now to your commit that "fixed" it. You have dropped out a optimization, > which was done to keep list duplication and clonging number low, without > even asking or trying to understand why. This was a lot of perf and > massif work to get those things down and small while working correctly. > Then you now went for the safe way and added if (manager) in every > single place where i called the order_update api. So #3 doesnt even > strike anymore, in the end those ifs where enough, without dropping out > the performance optimization. > > All in all this feels just like another "I need to fix this now without > asking why things where done that way". Could we somehow establish to at > least try to ask the person that wrote that code, and somehow coordinate > a fix that doesnt just hammer things in with a "deal with it" manner? > > Also on a slightly other note, just because code leaks memory or crashes > doesnt always indicate that we have a error on the called code. Thse > usecase where this elm.box was used is completly wrong and even tells > you that in a error messages. > > slightly salty greetings, > Marcel Hollerbach > > > > > > > > > Greetings > > > Marcel Hollerbach > > > > > > > } > > > > > > > > static void * > > > > diff --git a/src/lib/elementary/elm_grid.c > b/src/lib/elementary/elm_grid.c > > > > index 7c85648882..913ffe3b2f 100644 > > > > --- a/src/lib/elementary/elm_grid.c > > > > +++ b/src/lib/elementary/elm_grid.c > > > > @@ -20,6 +20,7 @@ _focus_order_flush(Eo *obj) > > > > Eina_List *order = evas_object_grid_children_get( > wpd->resize_obj); > > > > > > > > efl_ui_focus_manager_calc_update_order(wpd->focus.manager, obj, > order); > > > > + eina_list_free(order); > > > > } > > > > > > > > > > > > diff --git a/src/lib/elementary/elm_table.c > b/src/lib/elementary/elm_table.c > > > > index 48e6dfa62a..48bc137b23 100644 > > > > --- a/src/lib/elementary/elm_table.c > > > > +++ b/src/lib/elementary/elm_table.c > > > > @@ -21,6 +21,7 @@ _focus_order_flush(Eo *obj) > > > > Eina_List *order = evas_object_table_children_ > get(wpd->resize_obj); > > > > > > > > efl_ui_focus_manager_calc_update_order(wpd->focus.manager, obj, > order); > > > > + eina_list_free(order); > > > > } > > > > > > > > EOLIAN static Eina_Bool > > > > > > > > -- > > > > > > > > > > > > > > ------------------------------------------------------------ > ------------------ > > > 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 > > > [email protected] > > > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel > > > > > > > > > -- > > ------------- Codito, ergo sum - "I code, therefore I am" -------------- > > The Rasterman (Carsten Haitzler) [email protected] > > > > ------------------------------------------------------------ > ------------------ > 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 > [email protected] > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel > ------------------------------------------------------------------------------ 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 [email protected] https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
