Bug 32661 - [GTK] G_OBJECT() cast is not necessary for signals connection and properties access
: [GTK] G_OBJECT() cast is not necessary for signals connection and properties ...
Status: RESOLVED FIXED
Product: WebKit
Classification: Unclassified
Component: WebKit Gtk
: 528+ (Nightly build)
: PC Mac OS X 10.5
: P2 Normal
Assigned To: Nobody
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-12-17 07:06 PST by Philippe Normand
Modified: 2010-01-04 03:06 PST (History)
3 users (show)

See Also:


Attachments
proposed patch (4.04 KB, patch)
2009-12-17 07:36 PST, Philippe Normand
no flags Details | Formatted Diff | Diff
updated patch (6.30 KB, patch)
2009-12-20 03:29 PST, Philippe Normand
xan.lopez: review+
Details | Formatted Diff | Diff
reverted SENTINEL (4.95 KB, patch)
2010-01-04 02:13 PST, Philippe Normand
eric: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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 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 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 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.