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 766386 - GrlSource resolution functions don't delare taking ownership of media param
GrlSource resolution functions don't delare taking ownership of media param
Status: RESOLVED FIXED
Product: grilo
Classification: Other
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: grilo-maint
grilo-maint
Depends on:
Blocks:
 
 
Reported: 2016-05-13 16:59 UTC by Adrien Plazas
Modified: 2016-06-27 12:21 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
A test program showing the error (1.64 KB, text/x-vala)
2016-05-13 16:59 UTC, Adrien Plazas 		  Details
grl-source: Make resolution funcs own 'media' param (1.39 KB, patch)
2016-05-13 17:00 UTC, Adrien Plazas 		committed Details | Review

Description Adrien Plazas 2016-05-13 16:59:16 UTC 
Created attachment 327810 [details]
A test program showing the error

The grl_source_resolve() and grl_source_resolve_sync() functions don't declare taking the ownership of the reference to media parameter from the caller even though they do, which ends up breaking the Vala code generation (the generated VAPI lacks the "owned" keyword in front of the media parameter) and ake them unusable in Vala.

I attached an example program showing the problem, without the patch I have this result:
    refs: 1
    refs: 2863311530
    Erreur de segmentation (core dumped)
Comment 1 Adrien Plazas 2016-05-13 17:00:30 UTC 
Created attachment 327811 [details] [review]
grl-source: Make resolution funcs own 'media' param

Declare the ownership of the 'media' param of grl_source_resolve() and
grl_source_resolve_sync() as fully transfered.

This is needed to avoid reference counting bugs in the Vala bindings.
Comment 2 Adrien Plazas 2016-05-13 17:01:51 UTC 
Here is the result with the patch:
    refs: 1
    refs: 1
(and after that everything prints as expected)
Comment 3 Bastien Nocera 2016-05-13 22:10:46 UTC 
Review of attachment 327811 [details] [review]:

Looks good.
Comment 4 Bastien Nocera 2016-05-13 22:12:12 UTC 
Could you please integrate the test program into a test case that would be compiled and run when Vala support is enabled?

Looks good to commit when we have a regression test for it.
Comment 5 Bastien Nocera 2016-06-27 12:21:33 UTC 
Attachment 327811 [details] pushed as 6d46eb2 - grl-source: Make resolution funcs own 'media' param