Re: [Qemu-devel] [PATCH v2 4/5] monitor: add object-add (QMP) and object_add (HMP) command

[Markus Armbruster]

Peter Crosthwaite <address@hidden> writes:

> On Tue, Dec 17, 2013 at 10:24 PM, Paolo Bonzini <address@hidden> wrote:
>> Il 17/12/2013 12:54, Peter Crosthwaite ha scritto:
>>>> +    visit_start_struct(opts_get_visitor(ov), &dummy, NULL, NULL, 0, &err);
>>>> +    if (error_is_set(&err)) {
>>>> +        goto out_clean;
>>>> +    }
>>>
>>> So I have been thinking about repeated if(error_is_set(&err)) { goto
>>> foo; } and how to reduce its verbosity in situations like this. Can it
>>> be solved with a simple semantic:
>>>
>>> "Error ** accepting APIs will perform no action if the Error **
>>> argument is already set."
>>
>> I think this is a case where verbosity <<< ease of use.
>>
>> In this case, the caller code is particularly simple, but what if I
>> needed to dereference the return value of the first called function, to
>> get the argument to the second?  You would still need an "if".
>>
>
> Yes thats right. This isn't going to work universally and callers will
> always have the responsibility of knowing whether they can continue or
> not. But it will help a lot for repetitive collections of similar
> independent functions calls. The ultimate example is probably the
> device tree API calls in hw/ppc/e500.c.
>
> I want to patch the device tree API to be nice and Error**ified (for
> my own future reasons) but I sure don't want to have to patch e500 to
> check every qemu_devtree_foo API call with these 3 LOC. TBH i'll
> probably just preserve current behavior using &error_abort in next rev
> of that series, but it should be possible to do less verbose
> caller-customized collective error handling in some way.

error_set() & friends currently have a "errp doesn't contain an error
already" precondition:

    if (errp == NULL) {
        return;
    }
    assert(*errp == NULL);

You could change the function contracts to ignore additional errors.
Theoretical drawback: in situations where that isn't intended,
programming errors no longer get caught.  If people actually care for
that enough to veto your change, you can try adding new functions for
use when it is intended.