Re: [Qemu-devel] [PATCH v10 07/30] qapi: Simplify error cleanup in test-qmp-*

[Eric Blake]

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