From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35880) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aiduI-0005JX-1X for qemu-devel@nongnu.org; Wed, 23 Mar 2016 04:12:47 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aiduE-0007Tu-SN for qemu-devel@nongnu.org; Wed, 23 Mar 2016 04:12:45 -0400 Received: from mx1.redhat.com ([209.132.183.28]:39735) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aiduE-0007ST-Mo for qemu-devel@nongnu.org; Wed, 23 Mar 2016 04:12:42 -0400 From: Markus Armbruster References: <1450176195-9066-1-git-send-email-caoj.fnst@cn.fujitsu.com> <1450176195-9066-2-git-send-email-caoj.fnst@cn.fujitsu.com> <87wpplqm3i.fsf@blackfin.pond.sub.org> <56F215C0.60203@cn.fujitsu.com> Date: Wed, 23 Mar 2016 09:12:39 +0100 In-Reply-To: <56F215C0.60203@cn.fujitsu.com> (Cao jin's message of "Wed, 23 Mar 2016 12:04:16 +0800") Message-ID: <87zitpd348.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH RFC v2 1/2] Add param Error** to msi_init() & modify the callers List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cao jin Cc: Marcel Apfelbaum , qemu-devel@nongnu.org, mst@redhat.com Cao jin writes: > On 03/02/2016 05:13 PM, Markus Armbruster wrote: >> This got lost over the Christmas break, sorry. >> >> Cc'ing Marcel for additional PCI expertise. >> >> Cao jin writes: >> >>> msi_init() is a supporting function in PCI device initialization, >>> in order to convert .init() to .realize(), it should be modified first. >> >> "Supporting function" doesn't imply "should use Error to report >> errors". HACKING explains: >> >> Use the simplest suitable method to communicate success / failure to >> callers. Stick to common methods: non-negative on success / -1 on >> error, non-negative / -errno, non-null / null, or Error objects. >> >> Example: when a function returns a non-null pointer on success, and it >> can fail only in one way (as far as the caller is concerned), returning >> null on failure is just fine, and certainly simpler and a lot easier on >> the eyes than propagating an Error object through an Error ** parameter. >> >> Example: when a function's callers need to report details on failure >> only the function really knows, use Error **, and set suitable errors. >> >> Do not report an error to the user when you're also returning an error >> for somebody else to handle. Leave the reporting to the place that >> consumes the error returned. >> > > Really appreciate your review, I just finished reading all the > comments and discussion. > > Seems pci_add_capability2()(commit cd9aa33e introduced) doesn`t follow > the new error reporting rule(report error while also return error). Misunderstanding? "Report an error to the user" means error_report() and such. error_setg() doesn't report to the user, it returns an error object to the caller. > So I am thinking, could we revert commit cd9aa33e, let > pci_add_capability() return error code and assert when out of pci > space, and let caller(only assigned device, others could ignore the > error) handle the error code(new a error object, propagate it) > > Hope to hear PCI Maintainer`s advice(So I don`t cc other in this round)