[webkit-reviews] review granted: [Bug 130252] Fix WebCore unused parameter warnings for WebKitGTK+ CMake build : [Attachment 226738] Patch

[bugzilla-daemon at webkit.org]

Carlos Garcia Campos <cgarcia at igalia.com> has granted Martin Robinson
<mrobinson at webkit.org>'s request for review:
Bug 130252: Fix WebCore unused parameter warnings for WebKitGTK+ CMake build
https://bugs.webkit.org/show_bug.cgi?id=130252

Attachment 226738: Patch
https://bugs.webkit.org/attachment.cgi?id=226738&action=review

------- Additional Comments from Carlos Garcia Campos <cgarcia at igalia.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=226738&action=review


Thanks!

> Source/WebCore/platform/gtk/GtkVersioning.c:91
> +    UNUSED_PARAM(srcX);
> +

Have you checked this isn't a bug? sounds weird we are not using srcX, but I
haven't looked at the code.

> Source/WebCore/platform/soup/SharedBufferSoup.cpp:44
> -void SharedBuffer::tryReplaceContentsWithPlatformBuffer(SharedBuffer*
newContents)
> +void SharedBuffer::tryReplaceContentsWithPlatformBuffer(SharedBuffer* /*
newContents */)

Maybe we can simply omit the parameter in this case.

> Source/WebCore/plugins/gtk/PluginViewGtk.cpp:500
> -void PluginView::plugAddedCallback(GtkSocket* socket, PluginView* view)
> +void PluginView::plugAddedCallback(GtkSocket*, PluginView* view)
>  {
>      ASSERT(socket);

This is going to fail in debug builds. I think we can remove the assert, the
callback will never be emitted with a NULL widget.

> Source/WebCore/plugins/gtk/gtk2xtbin.c:135
> +  UNUSED_PARAM(source_data);
> +  UNUSED_PARAM(timeout);

We can't omit the names here because this file is C?

> Source/WebCore/rendering/InlineTextBox.cpp:991
> +#if !ENABLE(CSS3_TEXT_DECORATION_SKIP_INK)
> +    UNUSED_PARAM(textPainter);
> +#endif

Why not adding an #else to the existing ifdef? because there's more than one in
the function body?