Bug 32661 – [GTK] G_OBJECT() cast is not necessary for signals connection and properties access

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Philippe Normand]

The MediaPlayerPrivateGstreamer.cpp has those casts at various places of the code.

[Philippe Normand]

Created attachment 45067 [details]
proposed patch

[GTK] G_OBJECT() cast is not necessary for signals connection and properties access
        https://bugs.webkit.org/show_bug.cgi?id=32661

        Removed useless calls to the G_OBJECT() macro.

        * platform/graphics/gtk/MediaPlayerPrivateGStreamer.cpp:
        (WebCore::MediaPlayerPrivate::hasVideo):
        (WebCore::MediaPlayerPrivate::hasAudio):
        (WebCore::MediaPlayerPrivate::setVolume):
        (WebCore::MediaPlayerPrivate::createGSTPlayBin):

[WebKit Review Bot]

Attachment 45067 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebCore/platform/graphics/gtk/MediaPlayerPrivateGStreamer.cpp:415:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/graphics/gtk/MediaPlayerPrivateGStreamer.cpp:423:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/graphics/gtk/MediaPlayerPrivateGStreamer.cpp:432:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/graphics/gtk/MediaPlayerPrivateGStreamer.cpp:928:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/graphics/gtk/MediaPlayerPrivateGStreamer.cpp:940:  Use 0 instead of NULL.  [readability/null] [5]
Total errors found: 5

[Christian Dywan]

Looks obviously correct. While you're at it, it's probably a good opportunity to use 0 since this is WebCore code.

[Philippe Normand]

Created attachment 45266 [details]
updated patch

Replaced the NULL occurences with SENTINEL.

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 45266 [details] without any errors.

[Xan Lopez]

Comment on attachment 45266 [details]
updated patch

r=me

[Philippe Normand]

Landed as r52447.

[Eric Seidel (no email)]

Why is SENTINAL a good thing here?  Seems silly to introduce a new NULL.  If you want to use NULL here, please answer my mail on webkit-dev:
https://lists.webkit.org/pipermail/webkit-dev/2009-December/011041.html

[Philippe Normand]

(In reply to comment #8)
> Why is SENTINAL a good thing here?  Seems silly to introduce a new NULL.  If
> you want to use NULL here, please answer my mail on webkit-dev:
> https://lists.webkit.org/pipermail/webkit-dev/2009-December/011041.html

Well yes it is silly ;) But if we use 0 instead of NULL in g_object_{get,set} we get compilation warning:

warning: missing sentinel in function call

[Eric Seidel (no email)]

Interesting.  Seems like we should file a bug to have check-webkit-style not warn in this particular case.  It would be great if you could both reply to the thread and file a bug about such. :)

[Philippe Normand]

Will send a patch to remove the SENTINEL stuff introduced by this patch. It is not necessary anymore as the style bot won't complain about use of NULL in g_object_get/set calls.

[Philippe Normand]

Created attachment 45787 [details]
reverted SENTINEL

[Eric Seidel (no email)]

Comment on attachment 45787 [details]
reverted SENTINEL

OK.

[Philippe Normand]

Landed as r52727.