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
: WebKit
WebKit Gtk
: 528+ (Nightly build)
: PC Mac OS X 10.5
: P2 Normal
Assigned To:
:
:
:
:
  Show dependency treegraph
 
Reported: 2009-12-17 07:06 PST by
Modified: 2010-01-04 03:06 PST (History)


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


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2009-12-17 07:06:51 PST
The MediaPlayerPrivateGstreamer.cpp has those casts at various places of the code.
------- Comment #1 From 2009-12-17 07:36:30 PST -------
Created an attachment (id=45067) [details]
        Reviewed by NOBODY (OOPS!).

[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 From 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 From 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 From 2009-12-20 03:29:21 PST -------
Created an attachment (id=45266) [details]
        Reviewed by NOBODY (OOPS!).

Replaced the NULL occurences with SENTINEL.
------- Comment #5 From 2009-12-20 03:34:04 PST -------
style-queue ran check-webkit-style on attachment 45266 [details] without any errors.
------- Comment #6 From 2009-12-21 09:44:58 PST -------
(From update of attachment 45266 [details])
r=me
------- Comment #7 From 2009-12-21 09:59:54 PST -------
Landed as r52447.
------- Comment #8 From 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 From 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 From 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 From 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 From 2010-01-04 02:13:04 PST -------
Created an attachment (id=45787) [details]
reverted SENTINEL
------- Comment #13 From 2010-01-04 02:22:28 PST -------
(From update of attachment 45787 [details])
OK.
------- Comment #14 From 2010-01-04 03:06:18 PST -------
Landed as r52727.