GNOME Bugzilla – Bug 616846
Crash in gst_interpolation_control_source_find_control_point_iter
Last modified: 2010-04-27 09:24:29 UTC
This is a forward of https://bugzilla.redhat.com/show_bug.cgi?id=585743 A stack trace can be found at https://bugzilla.redhat.com/attachment.cgi?id=409012 This crash in Pitivi seemed to be caused by an empty, but existing control point list. I have no idea how such a thing can be created (nor much clue about how GstController works at all), so I didn't create a testcase. I do know how GSequence work, so I'll attached an untested, but compiling patch.
Created attachment 159598 [details] [review] controller: Fix gst_interpolation_control_source_find_control_point_iter The logic in that function is broken. Various NULL-checking bandaids for guaranteed non-NULL variables didn't even help there. This patch updates the function to check if a previous item exists before fetching it instead of after. This makes all other tests unnecessary. In particular, it makes the check for an empty list unnecessary, because for empty lists the only iter is the begin iter (and the end iter) and so the new check catches that case.
Looks good, can we get this into the next pre-release Tim?
Yes, please commit. A unit test would still be nice though.
Comment on attachment 159598 [details] [review] controller: Fix gst_interpolation_control_source_find_control_point_iter commit 6fb4cda8fb84c203d562adfe3eb9ed37218c42f8 Author: Benjamin Otte <otte@redhat.com> Date: Mon Apr 26 15:43:17 2010 +0200 controller: Fix gst_interpolation_control_source_find_control_point_iter The logic in that function is broken. Various NULL-checking bandaids for guaranteed non-NULL variables didn't even help there. This patch updates the function to check if a previous item exists before fetching it instead of after. This makes all other tests unnecessary. In particular, it makes the check for an empty list unnecessary, because for empty lists the only iter is the begin iter (and the end iter) and so the new check catches that case. https://bugzilla.gnome.org/show_bug.cgi?id=616846
I was trying to write a test for it. I will defer updating my core until I can reproduce it as this has been already commited.
(In reply to comment #5) > I was trying to write a test for it. I will defer updating my core until I can > reproduce it as this has been already commited. In case that wasn't obvious yet: git checkout -b before-fix 6fb4cda8fb84c203d562adfe3eb9ed37218c42f8 ... create testcase and commit it. git rebase master git checkout master git merge before-fix git branch -d before-fix And of course you can use cherry-pick to move commits (like the testcase) between master and the test branch at any time.
Company, thanks the patch works fine. commit bbda261b510143a1bc023b750ef5ce558b560f80 Author: Stefan Kost <ensonic@users.sf.net> Date: Tue Apr 27 09:42:05 2010 +0300 tests: add more tests for controller The tests verify that bug #616846 is indeed fixed.