From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54484) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aia0G-0004aD-Cy for qemu-devel@nongnu.org; Wed, 23 Mar 2016 00:02:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aia0D-0002Lm-56 for qemu-devel@nongnu.org; Wed, 23 Mar 2016 00:02:40 -0400 Received: from [59.151.112.132] (port=7547 helo=heian.cn.fujitsu.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aia0C-0002ER-PG for qemu-devel@nongnu.org; Wed, 23 Mar 2016 00:02:37 -0400 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> From: Cao jin Message-ID: <56F215C0.60203@cn.fujitsu.com> Date: Wed, 23 Mar 2016 12:04:16 +0800 MIME-Version: 1.0 In-Reply-To: <87wpplqm3i.fsf@blackfin.pond.sub.org> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit 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: Markus Armbruster Cc: Marcel Apfelbaum , qemu-devel@nongnu.org, mst@redhat.com 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). 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) -- Yours Sincerely, Cao jin