BALATON Zoltan <bala...@eik.bme.hu> writes:
> On Thu, 2 Apr 2020, Markus Armbruster wrote:
>> Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> writes:
>>> 02.04.2020 12:36, BALATON Zoltan wrote:
[...]
>>>> Not much better. Could it be something like:
[...]
>>>> ERRP_RET(object_property_set_link(cpuobj,
>>>> OBJECT(&s->cpu_container[i]),
>>>> "memory", errp));
>>>> ERRP_RET(object_property_set_link(cpuobj, OBJECT(s), "idau", errp));
>>>> ERRP_RET(object_property_set_bool(cpuobj, true, "realized", errp));
>>>>
>>>
>>> and turn all
>>>
>>> ret = func(...);
>>> if (ret < 0) {
>>> return ret;
>>> }
>>>
>>> into
>>>
>>> FAIL_RET(func(...))
>>>
>>> ?
>>>
>>> Not a problem to make such macro.. But I think it's a bad idea to turn all
>>> the code
>>> into sequence of macro invocations. It's hard to debug and follow.
>>
>> Yes. Hiding control flow in macros is almost always too much magic.
>> There are exceptions, but this doesn't look like one.
>
> I did't like this idea of mine too much either so I agree but I see no
> other easy way to simplify this. If you propose changing function
> return values maybe these should return errp instead of passing it as
> a func parameter? Could that result in simpler code and less macro
> magic needed?
I don't think so. Let's compare a few error handling patterns from
error.h.
* Call a function and receive an error from it:
* Error *err = NULL;
* foo(arg, &err);
* if (err) {
* handle the error...
* }
Making foo() return the error object instead of void looks like an
obvious win:
if (foo(arg)) {
handle the error...
}
Except it does not work, because surely a use of @err is hiding in
"handle the error" (or else we'd leak it). So you actually need
err = foo(arg)
if (err) {
handle the error...
}
All this saves you is initializing @err.
You can save a bit more
if ((err = foo(arg))) {
handle the error...
}
We generally frown upon assignemnts within if controlling expressions.
Next:
* Receive an error and pass it on to the caller:
[...]
* But when all you do with the error is pass it on, please use
* foo(arg, errp);
* for readability.
The obvious conversion
*errp = foo(arg);
is wrong for !errp, errp == &error_fatal, and errp == &error_abort.
You'd need
err = foo(arg);
error_propagate(errp, err);
or, if you're so inclined
error_propagate(errp, foo(arg));
Less legible, and it creates and destroys error objects where the
current method doesn't.
Returning the error object is even less attractive for functions that
now return values.
