Re: [Qemu-devel] chardev's and fd's in monitors

[Markus Armbruster <armbru@redhat.com>]

"Daniel P. Berrange" <berrange@redhat.com> writes:

> On Thu, Oct 20, 2016 at 12:01:08PM +0100, Dr. David Alan Gilbert wrote:
>> * Markus Armbruster (armbru@redhat.com) wrote:
>> > "Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:
>> 
>> > > The problem is that our way of propagating errors actively discourages 
>> > > people from adding errors and you're left with lots of useless invented errno's.
>> > > error_report makes it easy for people to scatter meaningful error messages
>> > > in at any point.
>> > >
>> > > Make an easy, concise way of reporting an error that fits in with
>> > > a propagation scheme and I'd consider converting stuff.
>> > 
>> > error_setg(errp, "This is as simple as it gets, I'm afraid")
>> > 
>> > Snark aside, I acknowledge the pain of converting call chains to
>> > propagate Error objects, having converted "a few" myself.
>> 
>> If you can get it down to that line it would be great!
>> 
>> But unfortunately it isn't:
>>   a) I have to make sure my cleanup path after that error_setg doesn't
>>      cause any other errors because that breaks the rule
>>      that I can't call error_setg twice.

We disallowed setting a second error because that's a common symptom of
forgotten or botched error handling.

For what it's worth, g_error_set() does the same.

>>   b) I've got to put the whole local_err/error_propagate stuff all over.

Only when you need to examine the errors you're propagating.

>>   c) We insist on allowing the Err ** pointer to be NULL that
>>      removes some potential simplifications.
>
> This last point is one i really dislike - if we could mandate that
> the error parameters was non-NULL, it will eliminate alot of the
> need to use local_err/error_propagate, as you could just check
> if (*errp), and so largely solve (b).

Outlawing null errp shifts the pain to the places that ignore errors.
Whether that would be a win is not obvious.  A patch doing the change
might demonstrate it is, but it would be a lot of work, possibly for
naught.

For what it's worth, g_error_set() permits null, too.