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 752790 - Invalid refcounting in Gst::Element::post_message()
Invalid refcounting in Gst::Element::post_message()
Status: RESOLVED FIXED
Product: gstreamermm
Classification: Bindings
Component: general
git master
Other Linux
: Normal normal
: ---
Assigned To: gstreamermm-maint
gstreamermm-maint
Depends on:
Blocks:
 
 
Reported: 2015-07-23 16:55 UTC by Michał Wróbel
Modified: 2016-10-03 14:17 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Gst::Element: fix refcounting in post_message() (6.12 KB, patch)
2015-07-23 16:57 UTC, Michał Wróbel
none Details | Review

Description Michał Wróbel 2015-07-23 16:55:05 UTC
Gst::Element::post_message() should take ownership of the passed message because gst_element_post_message() takes the message argument with transfer full convention. This leads to premature destruction of a message posted through this method.

Since gstreamermm now requires C++11, maybe it's a good time to fix it using rvalue reference arguments?
Comment 1 Michał Wróbel 2015-07-23 16:57:41 UTC
Created attachment 308003 [details] [review]
Gst::Element: fix refcounting in post_message()
Comment 2 Marcin Kolny (IRC: loganek) 2015-08-07 11:23:09 UTC
Review of attachment 308003 [details] [review]:

::: tools/m4/convert_gst.m4
@@ +193,3 @@
 _CONVERSION(`GstMessage*',`Glib::RefPtr<const Gst::Message>',`Glib::wrap($3)')
 _CONVERSION(`const Glib::RefPtr<Gst::Message>&',`GstMessage*', `Glib::unwrap($3)')
+_CONVERSION(`Glib::RefPtr<Gst::Message>&&',`GstMessage*',`Gst::unwrap_rvalue(std::move($3))')

RefPtr class contains release(), which should be used in unwrap_rvalue. However, as far as we use unwrap_rvalue only in conversion methods, we'd replace it with inline code, something like that:
  (($3) ? $3.release()->gobj() : nullptr).
Comment 3 Marcin Kolny (IRC: loganek) 2015-08-08 12:08:54 UTC
I pushed your patch (slightly modified).
Comment 4 Tomasz 2016-10-03 13:48:06 UTC
Why `($3) ? $3.release()->gobj() : nullptr' is better than `Gst::unwrap_rvalue(std::move($3))'?

Doesn't the second one improve readability? I wanted to replace all these trynary-inline-conversion with unwrap_rvalue.
Comment 5 Marcin Kolny (IRC: loganek) 2016-10-03 14:17:33 UTC
(In reply to Tomasz from comment #4)
> Why `($3) ? $3.release()->gobj() : nullptr' is better than
> `Gst::unwrap_rvalue(std::move($3))'?
> 
> Doesn't the second one improve readability? I wanted to replace all these
> trynary-inline-conversion with unwrap_rvalue.

Sure, we can do that.