[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





--- Comment #27 from Sergio Correia <sergio.correia at openbossa.org>  2013-05-24 00:09:56 PST ---
(In reply to comment #26)
> (From update of attachment 201709 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=201709&action=review
> 
> 
> 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.


It seemed to make sense to do this specialization for char*, but I see your point here. Indeed, things like this could introduce near-impossible to debug problems.

> 
> > 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?
> 

After your suggestion I did some experiments with it and I am soon going to post a patch for review.

> > 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.

Ideally, yes, since it's only used there. However, there is a comment below warning not to perform memory allocation in the middle of the fork()/exec(), due to potential deadlock risks, and hence the allocation part is done in the parent process.


Thanks for the your time, Benjamin!

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