After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 722021 - src/librygel-renderer-gst/rygel-playbin-player.vala: non-void function should return a value
src/librygel-renderer-gst/rygel-playbin-player.vala: non-void function should...
Status: RESOLVED FIXED
Product: rygel
Classification: Applications
Component: general
git master
Other Linux
: Normal normal
: ---
Assigned To: rygel-maint
rygel-maint
Depends on:
Blocks:
 
 
Reported: 2014-01-12 03:43 UTC by Ting-Wei Lan
Modified: 2014-02-23 16:30 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
renderer-gst: Deprecate element wrapping (1.12 KB, patch)
2014-02-09 11:58 UTC, Jens Georg
committed Details | Review
renderer-gst: Return null in error case (1.32 KB, patch)
2014-02-23 16:29 UTC, Jens Georg
committed Details | Review

Description Ting-Wei Lan 2014-01-12 03:43:27 UTC
In src/librygel-renderer-gst/rygel-playbin-player.vala, the function Player.wrap use return_if_fail, which should be replaced by return_val_if_fail.
Comment 1 Jussi Kukkonen 2014-01-12 11:08:36 UTC
I think we can remove the first check:
        return_if_fail (playbin != null);
Vala does this already.

The second one is trickier... I think returning early in a constructor leads to the object being leaked -- this happens with current code as well of course.
Comment 2 Jens Georg 2014-01-12 16:53:12 UTC
Yeah, that leaks, looking at the C code.
Comment 3 Jens Georg 2014-01-13 09:43:17 UTC
Although I would like to remove that constructor. Wrapping random pipelines doesn't really work well.
Comment 4 Jens Georg 2014-02-09 11:58:28 UTC
Created attachment 268569 [details] [review]
renderer-gst: Deprecate element wrapping

The function causes leaks in the error case and secondly, wrapping an existing
pipeline is not as helpful as I thought it would be.

Signed-off-by: Jens Georg <mail@jensge.org>
Comment 5 Jens Georg 2014-02-09 12:18:33 UTC
Attachment 268569 [details] pushed as d276d78 - renderer-gst: Deprecate element wrapping
Comment 6 Ting-Wei Lan 2014-02-09 12:56:19 UTC
The patch does not fix the problem. I still get error when compiling rygel with clang.
Comment 7 Jens Georg 2014-02-10 07:36:50 UTC
Ehm, you somehow omitted the part in the bug description that it fails to compile for you.
Comment 8 Ting-Wei Lan 2014-02-10 08:39:39 UTC
"non-void function should return a value" is an error in clang.
Comment 9 Jens Georg 2014-02-23 16:27:16 UTC
actually, return_val_if_fail doesn't leak because it happens before the g_object_new
Comment 10 Jens Georg 2014-02-23 16:29:32 UTC
Created attachment 270056 [details] [review]
renderer-gst: Return null in error case

This is slightly bad since this is a constructor; we can safely return here
because the C code has not executed the constructor yet.

As the function was deprecated anyway with an earlier commit, beaty doesn't
really matter.

Signed-off-by: Jens Georg <mail@jensge.org>
Comment 11 Jens Georg 2014-02-23 16:29:56 UTC
Attachment 270056 [details] pushed as 58c3ba9 - renderer-gst: Return null in error case