[webkit-reviews] review granted: [Bug 173655] [GStreamer][MiniBrowser] Honor GStreamer command line parameters in MiniBrowser : [Attachment 334147] Patch

[bugzilla-daemon at webkit.org]

Xabier Rodríguez Calvar <calvaris at igalia.com> has granted Philippe Normand
<pnormand at igalia.com>'s request for review:
Bug 173655: [GStreamer][MiniBrowser] Honor GStreamer command line parameters in
MiniBrowser
https://bugs.webkit.org/show_bug.cgi?id=173655

Attachment 334147: Patch

https://bugs.webkit.org/attachment.cgi?id=334147&action=review




--- Comment #27 from Xabier Rodríguez Calvar <calvaris at igalia.com> ---
Comment on attachment 334147
  --> https://bugs.webkit.org/attachment.cgi?id=334147
Patch

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

I don't know if carlos has any other comments and needs to freely undo my r+,
but other that the nits I wrote, LGTM.

> Source/WebCore/platform/graphics/gstreamer/GStreamerUtilities.cpp:245
> +    char** nArgv;

nArgv -> newArgv or just argv

> Source/WebCore/platform/graphics/gstreamer/GStreamerUtilities.cpp:247
> +    size_t argcSizeSplit = parameters.size();

You don't need this variable

> Source/WebCore/platform/graphics/gstreamer/GStreamerUtilities.cpp:248
> +    nArgv = g_new0(char*, argcSizeSplit+1);

parameters.size() + 1

> Source/WebCore/platform/graphics/gstreamer/GStreamerUtilities.cpp:251
> +    char** nArgv;
> +    parameters.insert(0, "WebProcess");
> +    size_t argcSizeSplit = parameters.size();
> +    nArgv = g_new0(char*, argcSizeSplit+1);
> +
> +    for (unsigned i = 0; i < parameters.size(); i++)
> +	   nArgv[i] = g_strdup(parameters[i].utf8().data());

If I had to write this, I would:
1. make the function take const Vector&
2. g_new0 size + 2 instead of 1
3. g_strdup("WebProcess") to newArgs[0].
4. begin the loop at 1.