Bug 75464

Summary:

[EFL][WK2] Add ProcessLauncherEfl.cpp

Product:

WebKit

Reporter:

YoungTaeck Song <youngtaeck.song>

Component:

WebKit EFL

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

gyuyoung.kim, lucas.de.marchi, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Other

OS:

Linux

Bug Depends on:

Bug Blocks:

61838

Attachments:

Description	Flags
patch for ProcessLauncherEfl	none
fetch for ProcessLauncherEfl	none
fetch for ProcessLauncherEfl	none
fetch for ProcessLauncherEfl	none
Patch	none
Patch	none
Patch	none

YoungTaeck Song

Reported 2012-01-03 00:45:33 PST

Add first version of ProcessLauncherEfl.cpp including launchProcess() and terminateProcess().

Attachments
patch for ProcessLauncherEfl (4.13 KB, patch) 2012-01-03 03:48 PST, YoungTaeck Song	no flags	Details Formatted Diff Diff
fetch for ProcessLauncherEfl (4.12 KB, patch) 2012-01-05 03:04 PST, YoungTaeck Song	no flags	Details Formatted Diff Diff
fetch for ProcessLauncherEfl (4.05 KB, patch) 2012-01-11 05:11 PST, YoungTaeck Song	no flags	Details Formatted Diff Diff
fetch for ProcessLauncherEfl (4.04 KB, patch) 2012-01-20 00:37 PST, YoungTaeck Song	no flags	Details Formatted Diff Diff
Patch (4.20 KB, patch) 2012-02-26 22:34 PST, YoungTaeck Song	no flags	Details Formatted Diff Diff
Patch (4.20 KB, patch) 2012-02-26 23:15 PST, YoungTaeck Song	no flags	Details Formatted Diff Diff
Patch (4.20 KB, patch) 2012-03-15 21:42 PDT, YoungTaeck Song	no flags	Details Formatted Diff Diff
Show Obsolete (6) View All Add attachment proposed patch, testcase, etc.

YoungTaeck Song

Comment 1 2012-01-03 03:48:31 PST

Created attachment 120928 [details] patch for ProcessLauncherEfl

Ryuan Choi

Comment 2 2012-01-04 22:53:02 PST

Comment on attachment 120928 [details] patch for ProcessLauncherEfl View in context: https://bugs.webkit.org/attachment.cgi?id=120928&action=review > Source/WebKit2/UIProcess/Launcher/efl/ProcessLauncherEfl.cpp:39 > +static const char* WebProcessName = "WebProcess"; > +static const char* PluginProcessName = "PluginProcess"; Almost looks good to me. As a minor issue, I realized that static const char foo[] is better than static const char* because it can go to .rodata.

YoungTaeck Song

Comment 3 2012-01-05 03:02:35 PST

(In reply to comment #2) > (From update of attachment 120928 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=120928&action=review > > > Source/WebKit2/UIProcess/Launcher/efl/ProcessLauncherEfl.cpp:39 > > +static const char* WebProcessName = "WebProcess"; > > +static const char* PluginProcessName = "PluginProcess"; > > Almost looks good to me. > > As a minor issue, I realized that static const char foo[] is better than static const char* because it can go to .rodata. ==> Thanks. I fixed it in the next fetch.

YoungTaeck Song

Comment 4 2012-01-05 03:04:23 PST

Created attachment 121251 [details] fetch for ProcessLauncherEfl

Gyuyoung Kim

Comment 5 2012-01-10 23:57:05 PST

Comment on attachment 121251 [details] fetch for ProcessLauncherEfl View in context: https://bugs.webkit.org/attachment.cgi?id=121251&action=review > Source/WebKit2/UIProcess/Launcher/efl/ProcessLauncherEfl.cpp:55 > + const char* processName = 0; IMHO, it is not good to use mixed char* and String(#67 line) from the point of consistency.

YoungTaeck Song

Comment 6 2012-01-11 05:11:11 PST

Created attachment 122010 [details] fetch for ProcessLauncherEfl

YoungTaeck Song

Comment 7 2012-01-20 00:37:58 PST

Created attachment 123265 [details] fetch for ProcessLauncherEfl Because RunLoop was moved to the webcore, I submit the new patch.

Ryuan Choi

Comment 8 2012-02-26 22:20:07 PST

Comment on attachment 123265 [details] fetch for ProcessLauncherEfl View in context: https://bugs.webkit.org/attachment.cgi?id=123265&action=review > Source/WebKit2/UIProcess/Launcher/efl/ProcessLauncherEfl.cpp:29 > +#include <stdio.h> IMO, stdio.h is not necessary.

YoungTaeck Song

Comment 9 2012-02-26 22:34:58 PST

Created attachment 128954 [details] Patch

Andreas Kling

Comment 10 2012-02-26 23:00:14 PST

Comment on attachment 128954 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=128954&action=review > Source/WebKit2/UIProcess/Launcher/efl/ProcessLauncherEfl.cpp:70 > + ssize_t result = readlink("/proc/self/exe", readLinkBuffer, PATH_MAX); > + > + if (result == -1) > + executablePath = String("/usr/bin"); > + else { > + char* executablePathPtr = dirname(readLinkBuffer); > + executablePath = String(executablePathPtr); > + } readlink() doesn't append a '\0' character. dirname() will read uninitialized memory in the 'else' branch here.

YoungTaeck Song

Comment 11 2012-02-26 23:15:24 PST

Created attachment 128960 [details] Patch

YoungTaeck Song

Comment 12 2012-02-26 23:24:39 PST

(In reply to comment #10) > (From update of attachment 128954 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=128954&action=review > > > Source/WebKit2/UIProcess/Launcher/efl/ProcessLauncherEfl.cpp:70 > > + ssize_t result = readlink("/proc/self/exe", readLinkBuffer, PATH_MAX); > > + > > + if (result == -1) > > + executablePath = String("/usr/bin"); > > + else { > > + char* executablePathPtr = dirname(readLinkBuffer); > > + executablePath = String(executablePathPtr); > > + } > > readlink() doesn't append a '\0' character. dirname() will read uninitialized memory in the 'else' branch here. Thank you for your kind review. I fixed it at the next patch.

Early Warning System Bot

Comment 13 2012-03-07 19:30:01 PST

Comment on attachment 128960 [details] Patch Attachment 128960 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/11867064

Hajime Morrita

Comment 14 2012-03-15 21:25:19 PDT

Comment on attachment 128960 [details] Patch r- for qt-wk2 redness.

YoungTaeck Song

Comment 15 2012-03-15 21:42:53 PDT

Created attachment 132194 [details] Patch

YoungTaeck Song

Comment 16 2012-03-15 21:46:22 PDT

(In reply to comment #14) > (From update of attachment 128960 [details]) > r- for qt-wk2 redness. Thanks for your review. There is no relationship between this patch and qt-wk2. Please review again.

WebKit Review Bot

Comment 17 2012-03-16 04:41:20 PDT

Comment on attachment 132194 [details] Patch Clearing flags on attachment: 132194 Committed r110988: <http://trac.webkit.org/changeset/110988>

WebKit Review Bot

Comment 18 2012-03-16 04:41:25 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.