[Webkit-unassigned] [Bug 105156] [EFL] Process launcher uses system() for wrapping the WebProcess when using WEB_PROCESS_CMD_PREFIX

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=105156





--- Comment #17 from Christophe Dumez <dchris at gmail.com>  2013-05-13 05:25:36 PST ---
(From update of attachment 201462)
View in context: https://bugs.webkit.org/attachment.cgi?id=201462&action=review

> Source/WebKit2/UIProcess/Launcher/efl/ProcessLauncherEfl.cpp:38
> +template <> void deleteOwnedArrayPtr<char* const>(char* const* args)

Someone should confirm but shouldn't this be in wtf/ instead?

> Source/WebKit2/UIProcess/Launcher/efl/ProcessLauncherEfl.cpp:40
> +    if (args) {

We usually do the opposite in WebKit: Return early.

if (!arg)
    return;

> Source/WebKit2/UIProcess/Launcher/efl/ProcessLauncherEfl.cpp:63
> +    char** args = new char*[splitArgs.size() + 1]; // extra room for null

We should probably adopt here instead of the return statement. Comment should start with a capital letter and end with a '.'.

> Source/WebKit2/UIProcess/Launcher/efl/ProcessLauncherEfl.cpp:64
> +    size_t i, numArgs = splitArgs.size();

Having several statements on the same line is uncommon in WebKit.

> Source/WebKit2/UIProcess/Launcher/efl/ProcessLauncherEfl.cpp:65
> +    for (i = 0; i < numArgs; ++i) {

i can be declared here: size_t i = 0

> Source/WebKit2/UIProcess/Launcher/efl/ProcessLauncherEfl.cpp:68
> +        strcpy(args[i], param.data());

strncpy?

> Source/WebKit2/UIProcess/Launcher/efl/ProcessLauncherEfl.cpp:104
> +    OwnArrayPtr<char* const> args = createArgsArray(processCmdPrefix, executablePath, String::number(sockets[0]), pluginPath);

Looks much better indeed.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.