Re: [Qemu-devel] [PATCH v4 15/17] vfio/pci: Remove vfio_msix_early_setup returned value

[Markus Armbruster <armbru@redhat.com>]

Auger Eric <eric.auger@redhat.com> writes:

> Hi Markus,
>
> On 04/10/2016 15:05, Markus Armbruster wrote:
>> Eric Auger <eric.auger@redhat.com> writes:
>> 
>>> The returned value is not used anymore by the caller, vfio_realize,
>>> since the error now is stored in the error object. So let's remove it.
>>>
>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>>
>>> ---
>>>
>>> Logically we could do that job for all the functions now getting an
>>> Error object passed as a parameter to avoid duplicate information
>>> between the error content and the returned value. This requires to use
>>> a local error object in vfio_realize. So I am not sure this is worth
>>> the candle.
>> 
>> Matter of taste, yours is fine.
>> 
>> We used to recommend returing void instead of an error code when the
>> function sets and error.  More parsimonious in theory, more boiler-plate
>> in practice, so we accept either now.  Perhaps we should even recommend
>> returning an error code, but such a recommendation needs to come with
>> patches converting existing code to it.
>
> The risk is that if a programmer returns an error value without setting
> the errp he will get a sigsev on subsequent error_prepend().

Yes, the function contract becomes more complex, giving the programmer
another way to screw up.  Besides crashing, callers might also detect
success as failure, failure as success, and leak error objects.

I don't particularly like setting such traps, but:

1. the risk already exists for functions returning a distinct value on
failure, e.g. null on failure and non-null on success, and

2. I've gotten really tired of the extra error-checking boiler-plate
necessary for functions returning void.

[...]