Replace malloc/calloc/free functions calls to new/delete operator calls
We shouldn't just blindly do that, see e.g. bug 45735. What specific instances of malloc/free would you like to change?
Or is this only about WebKit EFL layer, not WebCore?
(In reply to comment #2) > Or is this only about WebKit EFL layer, not WebCore? AFAIK, this patch will focus on EFL layer. He will add [EFL] to Bug title.
(In reply to comment #2) > Or is this only about WebKit EFL layer, not WebCore? Yes, it is EFL only. I am sorry for confusion. :)
Any update ?
(In reply to comment #5) > Any update ? I am working on it.
Created attachment 124320 [details] First patch This patch: - does not change ewk_js file due to that changes for this file are very big. I would like to show this changes in separate patch for this bug or open a new bug. - does not change realloc operations (separate patch will be prepared).
Comment on attachment 124320 [details] First patch View in context: https://bugs.webkit.org/attachment.cgi?id=124320&action=review > Source/WebKit/efl/ewk/ewk_tiled_model.cpp:675 > + tileUnusedCache->entries.count--; I think this code should be reviewed by backing store guys.
(In reply to comment #8) > (From update of attachment 124320 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=124320&action=review > > > Source/WebKit/efl/ewk/ewk_tiled_model.cpp:675 > > + tileUnusedCache->entries.count--; > > I think this code should be reviewed by backing store guys. It is a very simple change. Patch looks very bad only.
Comment on attachment 124320 [details] First patch View in context: https://bugs.webkit.org/attachment.cgi?id=124320&action=review >>> Source/WebKit/efl/ewk/ewk_tiled_model.cpp:675 >>> + tileUnusedCache->entries.count--; >> >> I think this code should be reviewed by backing store guys. > > It is a very simple change. Patch looks very bad only. Ok, it looks this patch almost good. If jungjik and Kwanghyuk agree with this patch, this patch is r+ informally. I think we need to land this patch for EFL port's code quality. Jungjik and Kwanghyuk, could you review this patch?
Comment on attachment 124320 [details] First patch 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.
(In reply to comment #10) > (From update of attachment 124320 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=124320&action=review > > >>> Source/WebKit/efl/ewk/ewk_tiled_model.cpp:675 > >>> + tileUnusedCache->entries.count--; > >> > >> I think this code should be reviewed by backing store guys. > > > > It is a very simple change. Patch looks very bad only. > > Ok, it looks this patch almost good. If jungjik and Kwanghyuk agree with this patch, this patch is r+ informally. > I think we need to land this patch for EFL port's code quality. > > Jungjik and Kwanghyuk, could you review this patch? This part of the code is okay. I think the logic is the same.
Created attachment 129031 [details] Next patch Fixed Raphael's comments. Additional change: I had to left malloc for Ewk_Tile object due to compilation error when "new" was used: Source/WebKit/efl/ewk/ewk_tiled_model.cpp:189:16: error: uninitialized const member in ‘Ewk_Tile {aka struct _Ewk_Tile}’ using ‘new’ without new-initializer [-fpermissive] Source/WebKit/efl/ewk/ewk_tiled_backing_store.h:71:22: note: ‘_Ewk_Tile::width’ should be initialized Anyway, initialization of const fileds in Ewk_Tile looks very ugly. Do you have any good idea to fix this without use of base C++ features like constructor or private section?
Comment on attachment 129031 [details] Next patch View in context: https://bugs.webkit.org/attachment.cgi?id=129031&action=review > Source/WebKit/efl/ewk/ewk_tiled_backing_store.cpp:29 > +#include <OwnPtr.h> You also need PassOwnPtr.h for the implementation of adoptPtr().
(In reply to comment #13) > Anyway, initialization of const fileds in Ewk_Tile looks very ugly. Do you have any good idea to fix this without use of base C++ features like constructor or private section? Nope.
Created attachment 129201 [details] Fixed Raphael's comment
(In reply to comment #15) > (In reply to comment #13) > > Anyway, initialization of const fileds in Ewk_Tile looks very ugly. Do you have any good idea to fix this without use of base C++ features like constructor or private section? > > Nope. Maybe it is a good point to find a good solution for this and future similar "issues"?
Comment on attachment 129201 [details] Fixed Raphael's comment It finally looks OK to me :)
(In reply to comment #18) > (From update of attachment 129201 [details]) > It finally looks OK to me :) Thanks for infomal review. :)
Comment on attachment 129201 [details] Fixed Raphael's comment I hope these new-allocated structs have a constructor instead of memeset()-ed...
(In reply to comment #20) > (From update of attachment 129201 [details]) > I hope these new-allocated structs have a constructor instead of memeset()-ed... Currently, fileds are initialized when it is needed in functions that creates object and this patch doesn't changed this way. IMO, missing simple constructor that initialize structure fields could be an issue in EFL port.
Comment on attachment 129201 [details] Fixed Raphael's comment Clearing flags on attachment: 129201 Committed r109205: <http://trac.webkit.org/changeset/109205>
All reviewed patches have been landed. Closing bug.