Bug 70883 – [EFL] Use template functions instead of memory allocation of C functions

• Status: RESOLVED
• Resolution: INVALID
• Component: WebKit EFL

[Gyuyoung Kim]

This is a fourth step in order to be more compliant with WebKit coding style. Use memory template functions instead of malloc, calloc and realloc directly. It makes efl port is more close to C++ coding style.

[Gyuyoung Kim]

Created attachment 112826 [details]
Patch

[Grzegorz Czajkowski]

View in context: https://bugs.webkit.org/attachment.cgi?id=112826&action=review

> Source/WebKit/efl/ChangeLog:3
> +        [EFL}  Use template functions instead of memory allocation of C functions.

} -> ]

> Source/WebKit/efl/ewk/ewk_memory_allocation.h:27
> +

These  templates should be documented as internal ones. See please documentation of private functions in WebKit-EFL.

[Raphael Kubo da Costa (:rakuco)]

Looking at the patch, I don't see a real benefit in doing this.

The biggest gain I see here is that one will not need to type "static_cast" all that much. However, it will require everyone to use a weird idiom that, to me, neither looks closer to C++ nor is standard C (which also means newcomers to the code will certainly feel needlessly confused).

An improvement I can think of that is related to this is increasing the use of smart pointers in many functions in ewk, as well as just using new+delete when one is not returning pointers to API users.

The code as it stands should not go in IMHO.

[Kenneth Rohde Christiansen]

Comment on attachment 112826 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=112826&action=review

> Source/WebKit/efl/ewk/ewk_memory_allocation.h:28
> +template <typename struct_type> struct_type* eMalloc(size_t amount = 1)

Shouldn't these be inlined?

I would just call it enew or so, ie. keep it simple. And call the calling like enew0() like gtk is doing g_new0(). In GTK the realloc method is called g_renew().

> Source/WebKit/efl/ewk/ewk_tiled_backing_store.cpp:256
> +    request = eMalloc<Ewk_Tiled_Backing_Store_Pre_Render_Request>(1);

There is no point in writing the 1 here.

eMalloc<Ewk_Tiled_Backing_Store_Pre_Render_Request>()

does the same

[Kenneth Rohde Christiansen]

You are already using glib right? can't you use the g_new etc?

[Raphael Kubo da Costa (:rakuco)]

glib support is optional, and I don't think it should be required in order to make room for this change. I still favour smart pointers + new/delete wherever possible instead.

[Kenneth Rohde Christiansen]

Sure smart pointers are really the way to go.

[Gyuyoung Kim]

(In reply to comment #7)
> Sure smart pointers are really the way to go.

Hello Kenneth and Kubo,

Sorry for late reply, Recently, I had urgent work. I also think smart pointer is way to go. This is a temporary solution, and I think again this is not to have big benefit. If you agree with my opinion, I'd like to set "RESOLVED LATER" to this bug.

[Raphael Kubo da Costa (:rakuco)]

I'd just close it as invalid, unless you intend to retitle the bug report and use it to do something different and unrelated to the original intent.

[Gyuyoung Kim]

(In reply to comment #9)
> I'd just close it as invalid, unless you intend to retitle the bug report and use it to do something different and unrelated to the original intent.

Ok, let's try to make smart pointer. I close this bug as resolved invalid.