Re: [Qemu-devel] [PATCH] qemu-sockets: Fix assertion failure

[Markus Armbruster]

Kevin Wolf <address@hidden> writes:

> Am 06.03.2013 um 16:04 hat Paolo Bonzini geschrieben:
>> Il 06/03/2013 15:46, Laszlo Ersek ha scritto:
>> >> > We could assert(!error_is_set(errp)) if we wanted. As soon as you've got
>> >> > an Error, you must return instead of calling more functions with the
>> >> > same error pointer.
>> > I think Luiz would suggest (*) to receive any error into a
>> > NULL-initialized local_err pointer; do the logic above on local_err, and
>> > just before returning, error_propagate() it to errp.
>> > 
>> > (*) I hope you can see what I did there: if you disagree, you get to
>> > take that to Luiz, even though he didn't say anything. I'm getting
>> > better at working this list! :)
>> 
>> I agree with Laszlo.
>
> I don't really understand the difference. As long as the function
> doesn't depend on the Error object to be present (which it doesn't),
> isn't it semantically exactly the same?

I guess it is in this case (I didn't call your patch wrong).  However, I
figure that as soon as we go beyond utterly trivial with errors, it's
advisable to put them in local variables, and error_propagate() to the
parameter only at the end.  Messing around with errp parameters directly
feels error-prone, and I'm afraid even correct messing could easily lead
to incorrect messing, when folks imitate it without really understanding
what they're doing.

Yes, extra local variables make the error propagation code even bulkier.
No, I don't like it any more than you do.

> Also, Markus' reply makes me think that I should restrict myself to code
> areas where errors are reported as -errno. That one I understand at
> least...

I wish I could, but I stepped into the error swamp long ago, and the
muck is still sticking to my heels.

Whenever a simple error reporting mechanism such as non-null/null or
non-negative/-errno suffices, I very much recommend to use it.