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: | P2 | ||||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||||
Hardware: | Other | ||||||||||||||||||
OS: | Linux | ||||||||||||||||||
Bug Depends on: | |||||||||||||||||||
Bug Blocks: | 61838 | ||||||||||||||||||
Attachments: |
|
Description
YoungTaeck Song
2012-01-03 00:45:33 PST
Created attachment 120928 [details]
patch for ProcessLauncherEfl
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. (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. Created attachment 121251 [details]
fetch for ProcessLauncherEfl
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. Created attachment 122010 [details]
fetch for ProcessLauncherEfl
Created attachment 123265 [details]
fetch for ProcessLauncherEfl
Because RunLoop was moved to the webcore, I submit the new patch.
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. Created attachment 128954 [details]
Patch
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. Created attachment 128960 [details]
Patch
(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 on attachment 128960 [details] Patch Attachment 128960 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/11867064 Comment on attachment 128960 [details]
Patch
r- for qt-wk2 redness.
Created attachment 132194 [details]
Patch
(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 on attachment 132194 [details] Patch Clearing flags on attachment: 132194 Committed r110988: <http://trac.webkit.org/changeset/110988> All reviewed patches have been landed. Closing bug. |