From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45194) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bsPNK-0007bb-B5 for qemu-devel@nongnu.org; Fri, 07 Oct 2016 03:15:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bsPNH-0007Su-5o for qemu-devel@nongnu.org; Fri, 07 Oct 2016 03:15:21 -0400 Received: from mx1.redhat.com ([209.132.183.28]:46008) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bsPNG-0007Sk-V4 for qemu-devel@nongnu.org; Fri, 07 Oct 2016 03:15:19 -0400 From: Markus Armbruster References: <1475498477-2695-1-git-send-email-eric.auger@redhat.com> <1475498477-2695-16-git-send-email-eric.auger@redhat.com> <87a8ekfenp.fsf@dusky.pond.sub.org> <1b3f727f-352a-2d0b-de98-918adbe05ba0@redhat.com> Date: Fri, 07 Oct 2016 09:15:15 +0200 In-Reply-To: <1b3f727f-352a-2d0b-de98-918adbe05ba0@redhat.com> (Auger Eric's message of "Thu, 6 Oct 2016 18:09:33 +0200") Message-ID: <87poncaav0.fsf@dusky.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH v4 15/17] vfio/pci: Remove vfio_msix_early_setup returned value List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Auger Eric Cc: alex.williamson@redhat.com, qemu-devel@nongnu.org, eric.auger.pro@gmail.com Auger Eric writes: > Hi Markus, > > On 04/10/2016 15:05, Markus Armbruster wrote: >> Eric Auger 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 >>> >>> --- >>> >>> 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. [...]