Bug 75464 - [EFL][WK2] Add ProcessLauncherEfl.cpp
Summary: [EFL][WK2] Add ProcessLauncherEfl.cpp
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 61838
  Show dependency treegraph
 
Reported: 2012-01-03 00:45 PST by YoungTaeck Song
Modified: 2012-03-16 04:41 PDT (History)
3 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description YoungTaeck Song 2012-01-03 00:45:33 PST
Add first version of ProcessLauncherEfl.cpp including launchProcess() and terminateProcess().
Comment 1 YoungTaeck Song 2012-01-03 03:48:31 PST
Created attachment 120928 [details]
patch for ProcessLauncherEfl
Comment 2 Ryuan Choi 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.
Comment 3 YoungTaeck Song 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.
Comment 4 YoungTaeck Song 2012-01-05 03:04:23 PST
Created attachment 121251 [details]
fetch for ProcessLauncherEfl
Comment 5 Gyuyoung Kim 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.
Comment 6 YoungTaeck Song 2012-01-11 05:11:11 PST
Created attachment 122010 [details]
fetch for ProcessLauncherEfl
Comment 7 YoungTaeck Song 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.
Comment 8 Ryuan Choi 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.
Comment 9 YoungTaeck Song 2012-02-26 22:34:58 PST
Created attachment 128954 [details]
Patch
Comment 10 Andreas Kling 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.
Comment 11 YoungTaeck Song 2012-02-26 23:15:24 PST
Created attachment 128960 [details]
Patch
Comment 12 YoungTaeck Song 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.
Comment 13 Early Warning System Bot 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
Comment 14 Hajime Morrita 2012-03-15 21:25:19 PDT
Comment on attachment 128960 [details]
Patch

r- for qt-wk2 redness.
Comment 15 YoungTaeck Song 2012-03-15 21:42:53 PDT
Created attachment 132194 [details]
Patch
Comment 16 YoungTaeck Song 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.
Comment 17 WebKit Review Bot 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>
Comment 18 WebKit Review Bot 2012-03-16 04:41:25 PDT
All reviewed patches have been landed.  Closing bug.