[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v10 07/30] qapi: Simplify error cleanup in test-
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v10 07/30] qapi: Simplify error cleanup in test-qmp-* |
Date: |
Fri, 6 Nov 2015 09:32:27 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
On 11/06/2015 09:23 AM, Markus Armbruster wrote:
> Eric Blake <address@hidden> writes:
>
>> On 11/06/2015 08:40 AM, Markus Armbruster wrote:
>>> Eric Blake <address@hidden> writes:
>>>
>>>> By moving err into data, we can let test teardown take care
>>>> of cleaning up any collected error; it also gives us fewer
>>>> lines of code between repeated tests where init runs teardown
>>>> on our behalf.
>>>
>>> I think this paragraph is no longer valid: you aren't moving err
>>> anywhere in this version.
>>
>> D'oh. I scrubbed the code, but not the commit message. At least it was
>> a faithful split of the v9 version of the commit, before I reworked the
>> mechanism :)
>>
>> How about:
>>
>> We have several tests that perform multiple sub-actions that are
>> expected to fail. Asserting that an error occurred, then clearing it up
>> to prepare for the next action, turned into enough boilerplate that it
>> was sometimes forgotten,
>
> If you got suitable commit SHAs handy, lets insert some here.
A number of them were introduced in d88f5fd (look at
test-qmp-input-visitor.c:test_validate_fail_struct_nested(), for
example), and only barely cleaned up in 5/30 of this series. I'm not
finding other commits off-hand, though.
>
>> risking memory leak or invalidating the efforts
>> of the second action (passing a non-NULL err into a function is
>> generally a bad idea). Encapsulate the boilerplate into a single helper
>> function error_free_or_abort(), and consistently use it.
>
> Works for me.
>
>>>> Rather than duplicate code between .c files, I added a new
>>>> test-qmp-common.h. I debated about putting
>>>> error_free_or_abort() in error.h, but it seems like something
>>>> that is only useful for tests.
>>>
>>> Maybe, maybe not. I'd accept it into error.h.
>>
>> Do you want me to post a fixup patch that relocates it into error.h?
>
> I guess the result would be slightly simpler, so go ahead.
Coming up soon.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH v10 05/30] qapi: Plug leaks in test-qmp-*, (continued)
- [Qemu-devel] [PATCH v10 05/30] qapi: Plug leaks in test-qmp-*, Eric Blake, 2015/11/06
- [Qemu-devel] [PATCH v10 15/30] qapi-types: Simplify gen_struct_field[s], Eric Blake, 2015/11/06
- [Qemu-devel] [PATCH v10 16/30] qapi: Drop obsolete tag value collision assertions, Eric Blake, 2015/11/06
- [Qemu-devel] [PATCH v10 11/30] qapi: Provide nicer array names in introspection, Eric Blake, 2015/11/06
- [Qemu-devel] [PATCH v10 07/30] qapi: Simplify error cleanup in test-qmp-*, Eric Blake, 2015/11/06
- [Qemu-devel] [PATCH] fixup! qapi: Simplify error cleanup in test-qmp-*, Eric Blake, 2015/11/06
[Qemu-devel] [PATCH v10 06/30] qapi: Simplify non-error testing in test-qmp-*, Eric Blake, 2015/11/06
[Qemu-devel] [PATCH v10 13/30] qapi: Track simple union tag in object.local_members, Eric Blake, 2015/11/06
[Qemu-devel] [PATCH v10 17/30] qapi: Simplify QAPISchemaObjectTypeMember.check(), Eric Blake, 2015/11/06
[Qemu-devel] [PATCH v10 25/30] qapi: Hoist tag collision check to Variants.check(), Eric Blake, 2015/11/06