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

[Eric Blake]

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, 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.

> 
>> 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?

> 
>> Signed-off-by: Eric Blake <address@hidden>
> 
> Patch looks okay.
> 

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

signature.asc
Description: OpenPGP digital signature