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 739640 - tests : fix leaks in adder unit test
tests : fix leaks in adder unit test
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other All
: Normal minor
: 1.5.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-11-04 19:57 UTC by Danny Song (sedman)
Modified: 2014-11-05 18:10 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
fix leaks in adder unit test. (1.33 KB, patch)
2014-11-04 19:59 UTC, Danny Song (sedman)
committed Details | Review
fix leaks and remove unnecessary gst_event_ref in adder unit test (1.70 KB, patch)
2014-11-05 17:23 UTC, Danny Song (sedman)
rejected Details | Review

Description Danny Song (sedman) 2014-11-04 19:57:01 UTC
tests : fix leaks in adder unit test.

Wrong cleanup code fixed in adder unit test.
Please review.
Comment 1 Danny Song (sedman) 2014-11-04 19:59:12 UTC
Created attachment 289989 [details] [review]
fix leaks in adder unit test.

fix leaks in adder unit test.
Comment 2 Tim-Philipp Müller 2014-11-05 12:46:15 UTC
Comment on attachment 289989 [details] [review]
fix leaks in adder unit test.

Thanks for the patch. Mostly looks good to me, just a few minor comments: could you please change your name in the git commit message to a proper name with capitalization and spaces etc.

In test_place_twice() the event should just be sent without the _ref(), then there's no need to unref it in the end during clean-up (that pattern is only needed when it's sent inside a loop).

Also, I'm not sure why you marked this as severity=major, it's pretty much the least severe kind of issue there is :)
Comment 3 Danny Song (sedman) 2014-11-05 17:23:22 UTC
Created attachment 290040 [details] [review]
fix leaks and remove unnecessary gst_event_ref in  adder unit test

Thanks for your review.
You're talking about test_play_twice,right?:) 
Agree, gst_element_send_event doesn't have to use _ref() because it doesn't reuse after the call.

And I changed my name.
Comment 4 Tim-Philipp Müller 2014-11-05 18:07:54 UTC
Comment on attachment 290040 [details] [review]
fix leaks and remove unnecessary gst_event_ref in  adder unit test

Only problem is, this patch doesn't actually work, but errors out after criticals. we need to keep the event alive because it's re-used in the message callback, I missed that. Will merge your original patch then.
Comment 5 Tim-Philipp Müller 2014-11-05 18:10:35 UTC
commit 6ccef9d223d57c52b54603f1fc910e1be95d9b3f
Author: Danny Song <danny.song.ga@gmail.com>
Date:   Wed Nov 5 04:34:44 2014 +0900

    test : fix leaks in adder unit test
    
    https://bugzilla.gnome.org/show_bug.cgi?id=739640