[Webkit-unassigned] [Bug 105156] [EFL][WK2] 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


Benjamin Poulain <benjamin at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #201709|review?                     |review-
               Flag|                            |




--- Comment #26 from Benjamin Poulain <benjamin at webkit.org>  2013-05-21 20:06:25 PST ---
(From update of attachment 201709)
View in context: https://bugs.webkit.org/attachment.cgi?id=201709&action=review

> Source/WebKit2/UIProcess/Launcher/efl/ProcessLauncherEfl.cpp:49
> +namespace WTF {
> +template <> void deleteOwnedArrayPtr<char* const>(char* const* args)
> +{
> +    if (!args)
> +        return;
> +
> +    for (size_t i = 0; args[i]; ++i)
> +        delete[] args[i];
> +    delete[] args;
> +}
> +} // namespace WTF
> +

This should not be moved to WTF, this should not exist at all.

You can change behaviors of WebKit ADTs with traits and template arguments. But certainly _NOT_ by doing template specialization of internal structure.

By doing this, you are breaking the semantic of OwnArrayPtr. Anyone who modify this file later (and any reviewer checking patch) would assume the common well defined behavior. This would introduce subtle bugs and make reviews impossible.

> Source/WebKit2/UIProcess/Launcher/efl/ProcessLauncherEfl.cpp:52
> +static PassOwnArrayPtr<char* const> createArgsArray(const String& prefix, const String& executablePath, const String& socket, const String& pluginPath)

Why not just return a Vector<OwnArrayPtr<char* const>> here?

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

It looks like this should be in the if (!pid) branch.

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