Bug 67705 - [EFL] Add missing *void* parameter
Summary: [EFL] Add missing *void* parameter
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-09-07 04:58 PDT by Gyuyoung Kim
Modified: 2011-09-16 01:28 PDT (History)
4 users (show)

See Also:


Attachments
Proposed Patch (3.29 KB, patch)
2011-09-07 05:01 PDT, Gyuyoung Kim
lucas.de.marchi: review-
lucas.de.marchi: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gyuyoung Kim 2011-09-07 04:58:09 PDT
The APIs which don't have parameter need to have *void* parameter because EFL APIs tends to be C-coding style.
Comment 1 Gyuyoung Kim 2011-09-07 05:01:15 PDT
Created attachment 106573 [details]
Proposed Patch
Comment 2 Lucas De Marchi 2011-09-07 11:04:41 PDT
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-.
Comment 3 Gyuyoung Kim 2011-09-08 00:51:18 PDT
(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 ?
Comment 4 Raphael Kubo da Costa (:rakuco) 2011-09-08 07:33:25 PDT
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.
Comment 5 Lucas De Marchi 2011-09-08 16:39:13 PDT
(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)
Comment 6 Lucas De Marchi 2011-09-08 16:40:50 PDT
(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.
Comment 7 Gyuyoung Kim 2011-09-08 18:06:43 PDT
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.
Comment 8 Raphael Kubo da Costa (:rakuco) 2011-09-08 18:38:38 PDT
(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++"?
Comment 9 Gyuyoung Kim 2011-09-08 18:49:50 PDT
(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.
Comment 10 Raphael Kubo da Costa (:rakuco) 2011-09-08 21:35:21 PDT
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.
Comment 11 Gyuyoung Kim 2011-09-13 18:33:43 PDT
(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.
Comment 12 Raphael Kubo da Costa (:rakuco) 2011-09-13 18:37:41 PDT
OK, could you please close this report then?
Comment 13 Gyuyoung Kim 2011-09-13 21:52:12 PDT
(In reply to comment #12)
> OK, could you please close this report then?

Ok, I will do that.