[webkit-reviews] review denied: [Bug 75848] [EFL] Replace malloc/calloc/free to new/delete : [Attachment 124320] First patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sun Feb 26 18:54:29 PST 2012


Raphael Kubo da Costa <kubo at profusion.mobi> has denied Tomasz Morawski
<t.morawski at samsung.com>'s request for review:
Bug 75848: [EFL] Replace malloc/calloc/free to new/delete
https://bugs.webkit.org/show_bug.cgi?id=75848

Attachment 124320: First patch
https://bugs.webkit.org/attachment.cgi?id=124320&action=review

------- Additional Comments from Raphael Kubo da Costa <kubo at profusion.mobi>
View in context: https://bugs.webkit.org/attachment.cgi?id=124320&action=review


Looks good overall, but there are some minor things to work out and you need to
rebase it.

> Source/WebKit/efl/ewk/ewk_history.cpp:251
> +    item->title = 0;
> +    item->alternateTitle = 0;
> +    item->uri = 0;
> +    item->originalUri = 0;

How about memset(item, 0, sizeof(*item)) before setting core?

> Source/WebKit/efl/ewk/ewk_tiled_backing_store.cpp:478
> +    OwnPtr<Ewk_Tiled_Backing_Store_Item> item = adoptPtr(new
Ewk_Tiled_Backing_Store_Item);

It's safer to #include <OwnPtr.h> in this file.

> Source/WebKit/efl/ewk/ewk_tiled_matrix.cpp:202
> +    OwnPtr<Ewk_Tile_Matrix> tileMatrix = WTF::adoptPtr(new Ewk_Tile_Matrix);


PassOwnPtr.h adds a "using WTF::adoptPtr()" clause, so you can use just
adoptPtr the same way you use just OwnPtr.

> Source/WebKit/efl/ewk/ewk_view.cpp:641
> +    priv->settings.encodingDefault =
eina_stringshare_add(priv->pageSettings->defaultTextEncodingName().utf8().data(
));

I'd rather not commit unrelated changes in this patch.


More information about the webkit-reviews mailing list