[Webkit-unassigned] [Bug 72017] [EFL] Add NULL checks after memory allocation

[bugzilla-daemon at webkit.org]

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





--- Comment #9 from Grzegorz <g.czajkowski at samsung.com>  2011-12-06 07:50:00 PST ---
(In reply to comment #8)
> (From update of attachment 117409 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=117409&action=review
> 
> I still think it is not needed to check for all these conditions. Speaking of other bug reports, https://bugs.webkit.org/show_bug.cgi?id=65408#c12 still makes a lot of sense (even though the patch ended up being committed).
> 
> > Source/WebCore/platform/efl/ScrollbarEfl.cpp:162
> > +    Edje_Message_Float_Set* message = 0;
> > +    char* mem = 0;
> > +    if (mem = new char[sizeof(Edje_Message_Float_Set) + sizeof(double)])
> > +        message = new(mem) Edje_Message_Float_Set;
> 
> This ended up looking quite ugly due to Edje's brain-dead way of manipulating this data structure. My suggestion is to just use
>   Edje_Message_Float_Set* message = static_cast<Edje_Message_Float_Set*>(malloc(sizeof(Edje_Message_Float_set) + sizeof(double)));
>   if (!message) {
>     // yadda yadda
>   }
> and then free() message after sending it.
> 
> If a reviewer prefers the use of placement new here, I suggest the following:
>   char* mem = new char[sizeof(Edje_Message_Float_Set) * sizeof(double)];
>   Edje_Message_Float_Set* message = new(mem) Edje_Message_Float_Set;
> in this case, I don't even think it makes sense to check if the allocation fails, as new never returns NULL

Ok, I will skip NULL checks for pointers initialized by new. 

> 
> > Source/WebKit/efl/ewk/ewk_cookies.cpp:104
> > +        } else
> > +            CRITICAL("Could not allocate Ewk_Cookie.");
> 
> I still wonder if this is really needed. Assuming the hypothetical case in which malloc fails in the middle of the loop, you nonetheless continue trying to allocate other Ewk_Cookies to add them to the list and then return. I don't think this helps the caller all that much.


Do you prefer to stop loop in case of malloc failure and destroy (by free()) allocated memory? A similar solution in used in ewk_frame_source_get().

> 
> > Source/WebKit/efl/ewk/ewk_window_features.cpp:126
> > +    if (!window_features->core) {
> > +        CRITICAL("Could not allocate WebCore::WindowFeatures.");
> > +        return 0;
> > +    }
> 
> new never returns NULL.

Ok, thanks.

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