Bug 32661

Summary: [GTK] G_OBJECT() cast is not necessary for signals connection and properties access
Product: WebKit Reporter: Philippe Normand <pnormand>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: christian, eric, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
proposed patch
none
updated patch
xan.lopez: review+
reverted SENTINEL eric: review+

Description Philippe Normand 2009-12-17 07:06:51 PST
The MediaPlayerPrivateGstreamer.cpp has those casts at various places of the code.
Comment 1 Philippe Normand 2009-12-17 07:36:30 PST
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):
Comment 2 WebKit Review Bot 2009-12-17 07:38:12 PST
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
Comment 3 Christian Dywan 2009-12-20 02:01:47 PST
Looks obviously correct. While you're at it, it's probably a good opportunity to use 0 since this is WebCore code.
Comment 4 Philippe Normand 2009-12-20 03:29:21 PST
Created attachment 45266 [details]
updated patch

Replaced the NULL occurences with SENTINEL.
Comment 5 WebKit Review Bot 2009-12-20 03:34:04 PST
style-queue ran check-webkit-style on attachment 45266 [details] without any errors.
Comment 6 Xan Lopez 2009-12-21 09:44:58 PST
Comment on attachment 45266 [details]
updated patch

r=me
Comment 7 Philippe Normand 2009-12-21 09:59:54 PST
Landed as r52447.
Comment 8 Eric Seidel (no email) 2009-12-21 10:17:52 PST
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
Comment 9 Philippe Normand 2009-12-21 10:47:17 PST
(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
Comment 10 Eric Seidel (no email) 2009-12-21 10:56:56 PST
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. :)
Comment 11 Philippe Normand 2010-01-04 02:09:33 PST
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.
Comment 12 Philippe Normand 2010-01-04 02:13:04 PST
Created attachment 45787 [details]
reverted SENTINEL
Comment 13 Eric Seidel (no email) 2010-01-04 02:22:28 PST
Comment on attachment 45787 [details]
reverted SENTINEL

OK.
Comment 14 Philippe Normand 2010-01-04 03:06:18 PST
Landed as r52727.