[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH RFC v2 29/47] qapi: Replace dirty is_c_ptr() by

From: Eric Blake
Subject: Re: [Qemu-devel] [PATCH RFC v2 29/47] qapi: Replace dirty is_c_ptr() by method c_null()
Date: Thu, 30 Jul 2015 08:19:28 -0600
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0

On 07/29/2015 11:22 AM, Markus Armbruster wrote:
> Eric Blake <address@hidden> writes:
>> On 07/29/2015 02:32 AM, Markus Armbruster wrote:
>>>>> 2. We can leak retval only when qmp_FOO() returns non-null and local_err
>>>>>    is non-null.  This must not happen, because:
>>>>>    a. local_err must be null before the call, and
>>>>>    b. the call must not return non-null when it sets local_err.
>>>> We don't state that contract anywhere, but I doubt any of the qmp_FOO()
>>>> functions violate it, so it is worth making it part of the contract.
>>> It's a general Error API rule: set an error exactly on failure.  It
>>> applies to any function returning errors through an Error **errp
>>> parameter, and we generally don't bother to spell it out for the
>>> individual functions.
>>> The part that needs to be spelling out is what success and failure mean.
>>> A qmp_FOO() returning an object returns null on failure.

For qmp_FOO(), this is a reasonable contract.  But our very own
generated code does not follow these rules: visit_type_FOO() can assign
into *obj even when setting an error, if it encounters a parse error
halfway through the struct, leaving the caller responsible to still
clean up the mess if it wants to avoid a memory leak.

Maybe that means our generated code needs to be reworked to properly
clean up on a failed parse, such that *obj is guaranteed to be NULL if
an error is returned.  As a separate patch, of course.

Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

reply via email to

[Prev in Thread] Current Thread [Next in Thread]