[webkit-reviews] review denied: [Bug 70679] [GTK] Build fixes for glib 2.31 (current master) : [Attachment 112088] proposed patch

[bugzilla-daemon at webkit.org]

Martin Robinson <mrobinson at webkit.org> has denied Philippe Normand
<pnormand at igalia.com>'s request for review:
Bug 70679: [GTK] Build fixes for glib 2.31 (current master)
https://bugs.webkit.org/show_bug.cgi?id=70679

Attachment 112088: proposed patch
https://bugs.webkit.org/attachment.cgi?id=112088&action=review

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


> Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp:115
> +#if GLIB_CHECK_VERSION(2, 31, 0)
> +    g_cond_init(priv->data_cond);
> +    g_mutex_init(priv->buffer_mutex);
> +#else
>      priv->data_cond = g_cond_new();
>      priv->buffer_mutex = g_mutex_new();
> +#endif
>  }

This seems wrong. You are passing uninitialized pointers to g_cond_init. It
seems like you'll need to allocate a GCond structure first. Please use the WTF
allocator too. :) The same pattern should be used for the GMutex stuff.

Would it be easier to simply use WTF threading primitives here?

> Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp:253
> +#if GLIB_CHECK_VERSION(2, 31, 0)
> +	   g_cond_clear(priv->data_cond);
> +#else
>	   g_cond_free(priv->data_cond);

And here you would need to free the allocated data.