Bug 67705 – [EFL] Add missing *void* parameter

• Component: WebKit EFL
• Status: RESOLVED INVALID

[Gyuyoung Kim]

The APIs which don't have parameter need to have *void* parameter because EFL APIs tends to be C-coding style.

[Gyuyoung Kim]

Created attachment 106573 [details]
Proposed Patch

[Lucas De Marchi]

Comment on attachment 106573 [details]
Proposed Patch

void parameter is not a coding-style thing.

In C, a function with no parameters is achieved by putting a void keyword in the parameter's list. Without any keywords it's considered a function that can receive any number of parameters (the equivalent of writing (...)  in C++).

Although this is a valid function prototype in C++, it's not a the preferred way of doing it. Since these are all C++ files and header that is included only by C++ sources, there's no point in adding them at all.

See: http://en.wikipedia.org/wiki/Void_type

For the reasons pointed out above, my informal r-.

[Gyuyoung Kim]

(In reply to comment #2)
> (From update of attachment 106573 [details])
> void parameter is not a coding-style thing.
> 
> In C, a function with no parameters is achieved by putting a void keyword in the parameter's list. Without any keywords it's considered a function that can receive any number of parameters (the equivalent of writing (...)  in C++).
> 
> Although this is a valid function prototype in C++, it's not a the preferred way of doing it. Since these are all C++ files and header that is included only by C++ sources, there's no point in adding them at all.
> 
> See: http://en.wikipedia.org/wiki/Void_type
> 
> For the reasons pointed out above, my informal r-.

I understand what your point. However, as you know, real implementation of ewk is C, not C++. So, I don't know whether we should adhere C++ coding rule. Furthermore, almost existing implementations in ewk are using *void* parameter. In addition, it looks gtk port is also using void parameter.

3448 GtkWidget* webkit_web_view_new(void)
3449 {
3450     WebKitWebView* webView = WEBKIT_WEB_VIEW(g_object_new(WEBKIT_TYPE_WEB_VIEW, NULL));
3451 
3452     return GTK_WIDGET(webView);
3453 }

How do you think ?

[Raphael Kubo da Costa (:rakuco)]

ewk code is actually a mix of C++ and C, as variables are sometimes declared in the middle of functions and we have instantiate classes and call C++ code. IMO, we should not strive to be as C++-free as possible, as some of the language's features can be useful.

That said, using (void) should be really needed in public headers, as these are going to be included by C applications and not having it will cause the C compiler to accept multiple parameters instead of none. Implementation files and internal headers do not need that, as they are only seen by the C++ compiler.

[Lucas De Marchi]

(In reply to comment #3)
> (In reply to comment #2)
> > (From update of attachment 106573 [details] [details])
> > void parameter is not a coding-style thing.
> > 
> > In C, a function with no parameters is achieved by putting a void keyword in the parameter's list. Without any keywords it's considered a function that can receive any number of parameters (the equivalent of writing (...)  in C++).
> > 
> > Although this is a valid function prototype in C++, it's not a the preferred way of doing it. Since these are all C++ files and header that is included only by C++ sources, there's no point in adding them at all.
> > 
> > See: http://en.wikipedia.org/wiki/Void_type
> > 
> > For the reasons pointed out above, my informal r-.
> 
> I understand what your point. However, as you know, real implementation of ewk is C, not C++. So, I don't know whether we should adhere C++ coding rule. Furthermore, almost existing implementations in ewk are using *void* parameter. In addition, it looks gtk port is also using void parameter.

But it's compiled by a C++ compiler.

> 
> 3448 GtkWidget* webkit_web_view_new(void)
> 3449 {
> 3450     WebKitWebView* webView = WEBKIT_WEB_VIEW(g_object_new(WEBKIT_TYPE_WEB_VIEW, NULL));
> 3451 
> 3452     return GTK_WIDGET(webView);
> 3453 }
> 
> How do you think ?

In public header we must put the void keyword, however private headers and sources should use C++ if they are C++ sources (btw, we don't have any .c file anymore)

[Lucas De Marchi]

(In reply to comment #4)
> ewk code is actually a mix of C++ and C, as variables are sometimes declared in the middle of functions and we have instantiate classes and call C++ code. IMO, we should not strive to be as C++-free as possible, as some of the language's features can be useful.
> 
> That said, using (void) should be really needed in public headers, as these are going to be included by C applications and not having it will cause the C compiler to accept multiple parameters instead of none. Implementation files and internal headers do not need that, as they are only seen by the C++ compiler.

Yes... we have a C interface (the public headers) but the implementation is C++, just like is done for JSC, gtk, etc.

[Gyuyoung Kim]

Ok, I agree that ewk have to go C++ coding style in its implementation and internal header except for public header. As you know, ewk is using C and C++ mixed. It looks we need to change unneeded C coding with C++ in ewk implementation and private header. If you guys agree with this, I would like to prepare patches for it.

[Raphael Kubo da Costa (:rakuco)]

(In reply to comment #7)
> It looks we need to change unneeded C coding with C++ in ewk implementation and private header. If you guys agree with this, I would like to prepare patches for it.

Can you give some examples of what you have in mind when you say "change unneeded C coding with C++"?

[Gyuyoung Kim]

(In reply to comment #8)
> (In reply to comment #7)
> > It looks we need to change unneeded C coding with C++ in ewk implementation and private header. If you guys agree with this, I would like to prepare patches for it.
> 
> Can you give some examples of what you have in mind when you say "change unneeded C coding with C++"?

For example, I think implementation of ewk_settings.cpp uses void parameter. And, I mean I would like to make patches if I find unneeded C coding style.

[Raphael Kubo da Costa (:rakuco)]

I see.

Well, if these were new files I'd really like to have this kind of issue fixed before committing them. On the other hand, preparing big patches to fix these issues in existing files sounds a bit wasteful to me, in the sense that it's fixing what's not broken (depending on the patch, these stylistic fixes also make it hard to use svn blame, for example).

Personally, I would very much favor some of these issues being fixed "organically" as part of some real bug fix or code change.

[Gyuyoung Kim]

(In reply to comment #10)
> I see.
> 
> Well, if these were new files I'd really like to have this kind of issue fixed before committing them. On the other hand, preparing big patches to fix these issues in existing files sounds a bit wasteful to me, in the sense that it's fixing what's not broken (depending on the patch, these stylistic fixes also make it hard to use svn blame, for example).
> 
> Personally, I would very much favor some of these issues being fixed "organically" as part of some real bug fix or code change.

Because Korea is thanks giving day, my response is late. Ok. I agree with your opinion. Let's fix coding style together with other bug.

[Raphael Kubo da Costa (:rakuco)]

OK, could you please close this report then?

[Gyuyoung Kim]

(In reply to comment #12)
> OK, could you please close this report then?

Ok, I will do that.