[Webkit-unassigned] [Bug 61423] [EFL] Eina_List out of memory handling

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=61423





--- Comment #6 from Grzegorz <g.czajkowski at samsung.com>  2011-05-26 00:39:38 PST ---
Thanks for review. Sounds reasonably to separate OOM handling for each function. What will you say of following scenario:

func() {

   eina_list_append();
   if (eina_error_get())
      goto out_of_memory_handling_func;

// Out of memory handling for func()
out_of_memory_handling_func:
    CRITICAL("%s", eina_error_msg_get(eina_error_get()));
    void* _tmp_data;
    EINA_LIST_FREE(list, _tmp_data)
       method_for_free_item(_tmp_data) // like ewk_cookies_free(_tmp_data), ewk_context_menu_item_free(_tmp_data)
    return ;
}

A similar solution was proposed in html saving feature for OOM (bug id: 55455)

(In reply to comment #4)
> Informal r- from my side: in case the OOM handler is called, the memory still won't be properly freed. As the kind of destruction calls varies depending on what kind of data you are freeing, the macro may end up looking ugly enough not to have its existence justified. IMO, these checks could be done in-place in each file, or you can just ignore the eina_list_append errors, which is done 99% of the time in EFL code.
> 
> > Source/WebKit/efl/ewk/ewk_contextmenu.cpp:411
> > +    EINA_LIST_OUT_OF_MEMORY_HANDLING_AND_RETURN(o->items)
> 
> You are just calling free() on the pointer, but you still need to call ewk_context_menu_item_free in order not to leak the memory allocated in the items inside the struct.
> 
> > Source/WebKit/efl/ewk/ewk_cookies.cpp:120
> > +        EINA_LIST_OUT_OF_MEMORY_HANDLING_AND_RETURN(el, 0)
> 
> You are just calling free() on the pointer, but you are not freeing the memory allocated for the data in the struct it points to.
> 
> > Source/WebKit/efl/ewk/ewk_history.cpp:108
> > +            EINA_LIST_OUT_OF_MEMORY_HANDLING_AND_RETURN(ret, 0)
> 
> You are just calling free() on the pointer, but you are not freeing the memory allocated for the data (or de-ref'ing the data when necessary), which is done in _ewk_history_item_free().
> 
> > Source/WebKit/efl/ewk/ewk_private.h:49
> > +#define EINA_LIST_OUT_OF_MEMORY_HANDLING_AND_RETURN(list, ...)          \
> 
> The macro name looks a bit misleading, as you are not necessarily out of memory when there is an error.
> 
> > Source/WebKit/efl/ewk/ewk_view.cpp:4119
> > +        EINA_LIST_OUT_OF_MEMORY_HANDLING_AND_RETURN(priv->popup.menu.items)
> 
> The same comment I made to your changes to ewk_contextmenu.cpp applies here.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.