[Webkit-unassigned] [Bug 186841] [WPE] Pass the backend library name as command line parameter to the web process

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=186841

Zan Dobersek <zan at falconsigh.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #344682|review?                     |review+
              Flags|                            |

--- Comment #20 from Zan Dobersek <zan at falconsigh.net> ---
Comment on attachment 344682
  --> https://bugs.webkit.org/attachment.cgi?id=344682
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=344682&action=review

>>>>> Source/WebKit/UIProcess/Launcher/glib/ProcessLauncherGLib.cpp:106
>>>>> +#endif
>>>> 
>>>> Do we need the --backend-library flag?
>>> 
>>> Comment #8
>>> 
>>> "Yes, probably, I'm trying to avoid passing other stuff to wpe loader if we mess it up with the order of the arguments."
>>> 
>>> I can remove it if you don't like it.
>> 
>> It's out-of-place in that argument array, IMO.
>> 
>> Regardless of that, should the library name be escaped somehow before it's included in the arguments? Is there any way in which the impl library name alone could cause trouble? Should we use FileSystem::fileSystemRepresentation() like we do for other filenames of executables?
> 
> That's a good point, I'll use fileSystemRepresantation.

r=me with this change.

>>>>> Source/WebKit/UIProcess/Launcher/glib/ProcessLauncherGLib.cpp:137
>>>>> +        argv[i++] = wpeBackendLibraryParameter ? wpeBackendLibraryParameter.get() : "-";
>>>> 
>>>> I don't think wpeBackendLibraryParameter can be null if you use g_strdup_printf()? Unless we can remove the --backend-library flag and just strdup the impl library name, if any.
>>> 
>>> It can be nullptr when building with previous versions of WPEBackend.
>> 
>> Let's ensure it's non-null by assigning it the "-" token by default, and override it when compiling against the newer WPEBackend lib.
> 
> I don't see the point here. We would be heap allocating "-", just to free and allocate again in the case of new wpebackend, which will be the common one. I don't see the problem with the check here and using a static string "-".

Makes sense.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20180711/dd7aac86/attachment-0001.html>