[webkit-reviews] review denied: [Bug 48511] [GTK] Implement Process/Thread Launcher classes and WebProcessGtkMain for WebKit2 : [Attachment 78312] Addressed review comments

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat Jan 8 09:40:12 PST 2011


Martin Robinson <mrobinson at webkit.org> has denied Amruth Raj
<amruthraj at motorola.com>'s request for review:
Bug 48511: [GTK] Implement Process/Thread Launcher classes and
WebProcessGtkMain for WebKit2
https://bugs.webkit.org/show_bug.cgi?id=48511

Attachment 78312: Addressed review comments
https://bugs.webkit.org/attachment.cgi?id=78312&action=review

------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=78312&action=review

Thanks for the quick update! There's one more significant issue here that I can
see before landing.

> WebCore/platform/gtk/FileSystemGtk.cpp:196
> +    String filename = filenameToString(pathFromProc);
> +    return directoryName(filename).utf8();

Here you should just return pathFromProc. Sorry that I wasn't clear about that.
Converting a CString to a String directly assumes that it's in the latin1
encoding. And then using the UTF-8 encoding might break the subsequent call to
execl if the target filesystem is not UTF-8 encoded.

> WebCore/platform/gtk/FileSystemGtk.cpp:204
> +    String filename = filenameToString(currentExePath.get());
> +    return directoryName(fileName).utf8();

Please just return currentExePath here.


More information about the webkit-reviews mailing list