[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v6 20/26] qapi: Fix output visitor to return qnu
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v6 20/26] qapi: Fix output visitor to return qnull() instead of NULL |
Date: |
Sat, 12 Sep 2015 08:17:45 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 |
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
- [Qemu-devel] [PATCH v6 19/26] qapi: Improve built-in type documentation, (continued)
- [Qemu-devel] [PATCH v6 19/26] qapi: Improve built-in type documentation, Markus Armbruster, 2015/09/11
- [Qemu-devel] [PATCH v6 18/26] qapi-commands: De-duplicate output marshaling functions, Markus Armbruster, 2015/09/11
- [Qemu-devel] [PATCH v6 20/26] qapi: Fix output visitor to return qnull() instead of NULL, Markus Armbruster, 2015/09/11
- Re: [Qemu-devel] [PATCH v6 20/26] qapi: Fix output visitor to return qnull() instead of NULL, Eric Blake, 2015/09/11
- Re: [Qemu-devel] [PATCH v6 20/26] qapi: Fix output visitor to return qnull() instead of NULL, Eric Blake, 2015/09/11
- Re: [Qemu-devel] [PATCH v6 20/26] qapi: Fix output visitor to return qnull() instead of NULL, Markus Armbruster, 2015/09/12
- Re: [Qemu-devel] [PATCH v6 20/26] qapi: Fix output visitor to return qnull() instead of NULL, Eric Blake, 2015/09/12
- Re: [Qemu-devel] [PATCH v6 20/26] qapi: Fix output visitor to return qnull() instead of NULL, Eric Blake, 2015/09/12
- Re: [Qemu-devel] [PATCH v6 20/26] qapi: Fix output visitor to return qnull() instead of NULL, Eric Blake, 2015/09/12
- Re: [Qemu-devel] [PATCH v6 20/26] qapi: Fix output visitor to return qnull() instead of NULL, Markus Armbruster, 2015/09/12
- Re: [Qemu-devel] [PATCH v6 20/26] qapi: Fix output visitor to return qnull() instead of NULL,
Eric Blake <=
[Qemu-devel] [PATCH v6 15/26] qapi-commands: Rearrange code, Markus Armbruster, 2015/09/11
[Qemu-devel] [PATCH v6 26/26] qapi-introspect: Hide type names, Markus Armbruster, 2015/09/11
[Qemu-devel] [PATCH v6 24/26] qapi: Pseudo-type '**' is now unused, drop it, Markus Armbruster, 2015/09/11
[Qemu-devel] [PATCH v6 16/26] qapi: Rename qmp_marshal_input_FOO() to qmp_marshal_FOO(), Markus Armbruster, 2015/09/11
[Qemu-devel] [PATCH v6 21/26] qapi: Introduce a first class 'any' type, Markus Armbruster, 2015/09/11
[Qemu-devel] [PATCH v6 22/26] qom: Don't use 'gen': false for qom-get, qom-set, object-add, Markus Armbruster, 2015/09/11
[Qemu-devel] [PATCH v6 25/26] qapi: New QMP command query-qmp-schema for QMP introspection, Markus Armbruster, 2015/09/11