Auger Eric <eric.au...@redhat.com> writes: > Hi Markus, > > On 04/10/2016 15:05, Markus Armbruster wrote: >> Eric Auger <eric.au...@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.au...@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. [...]