RESOLVED FIXED 75464
[EFL][WK2] Add ProcessLauncherEfl.cpp
https://bugs.webkit.org/show_bug.cgi?id=75464
Summary [EFL][WK2] Add ProcessLauncherEfl.cpp
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
fetch for ProcessLauncherEfl (4.12 KB, patch)
2012-01-05 03:04 PST, YoungTaeck Song
no flags
fetch for ProcessLauncherEfl (4.05 KB, patch)
2012-01-11 05:11 PST, YoungTaeck Song
no flags
fetch for ProcessLauncherEfl (4.04 KB, patch)
2012-01-20 00:37 PST, YoungTaeck Song
no flags
Patch (4.20 KB, patch)
2012-02-26 22:34 PST, YoungTaeck Song
no flags
Patch (4.20 KB, patch)
2012-02-26 23:15 PST, YoungTaeck Song
no flags
Patch (4.20 KB, patch)
2012-03-15 21:42 PDT, YoungTaeck Song
no flags
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
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
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
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
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.