On 09/12/2015 08:11 AM, Markus Armbruster wrote: >> Indeed. Without reading further, we're both shooting in the dark for >> something that makes tests pass, but without being a clean interface. >> >> How about this: go ahead with your series as proposed, with the squash >> hunk to tests/ to avoid the leak in the testsuite, but leaving the leak >> in qmp_output_get_object(), and we address the leak in a followup patch.
I've given a couple more responses with my analysis, but up to you how much you want to take now or defer to later. > > I'll add a FIXME explaining the reference counting bug, and the wider > problem. > > What exactly do you want changed in tests? Definitely add the qobject_decref(arg). But the g_assert(refcnt...) should not be added unless we go with a solution that doesn't leak, and instead should be replaced with a FIXME, if you don't want my followup mails now. >> The followup patch(es) will then have to include some contract >> documentation, so that we track what we learned while investigating the >> code, and so that the next reader has more than just code to start from. > > It's time to retrofit visitors with a proper contract. > > Retrofitting a contract is much harder than starting with one, but we > got to play the hand we've been dealt. I've already started that work in some of my pending patches: https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg02597.html but it could indeed use further documentation in each visitor implementation. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
