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

[Markus Armbruster]

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.

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

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