From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=49403 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1P06oI-000505-Eo
	for qemu-devel@nongnu.org; Mon, 27 Sep 2010 02:03:35 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69)
	(envelope-from <yamahata@valinux.co.jp>) id 1P06oE-0005Dj-6m
	for qemu-devel@nongnu.org; Mon, 27 Sep 2010 02:03:34 -0400
Received: from mail.valinux.co.jp ([210.128.90.3]:48406)
	by eggs.gnu.org with esmtp (Exim 4.69)
	(envelope-from <yamahata@valinux.co.jp>) id 1P06oD-0005DF-JO
	for qemu-devel@nongnu.org; Mon, 27 Sep 2010 02:03:30 -0400
Date: Mon, 27 Sep 2010 15:03:24 +0900
From: Isaku Yamahata <yamahata@valinux.co.jp>
Message-ID: <20100927060324.GA28732@valinux.co.jp>
References: <cover.1284528424.git.yamahata@valinux.co.jp>
	<8d5f54a16ee43afe3280dd715659cd34fb1a7d44.1284528424.git.yamahata@valinux.co.jp>
	<20100922115016.GE16423@redhat.com>
	<20100924025021.GB4701@valinux.co.jp>
	<20100926124651.GF19655@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20100926124651.GF19655@redhat.com>
Subject: [Qemu-devel] Re: [PATCH v3 06/13] pcie/aer: helper functions for
	pcie aer capability.
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: skandasa@cisco.com, etmartin@cisco.com, qemu-devel@nongnu.org, wexu2@cisco.com

On Sun, Sep 26, 2010 at 02:46:51PM +0200, Michael S. Tsirkin wrote:
> > > > +static inline void pcie_aer_errmsg(PCIDevice *dev, const PCIE_AERErrMsg *msg)
> > > > +{
> > > > +    assert(pci_is_express(dev));
> > > > +    assert(dev->exp.aer_errmsg);
> > > > +    dev->exp.aer_errmsg(dev, msg);
> > > 
> > > Why do we want the indirection? Why not have users just call the function?
> > 
> > To handle error signaling uniformly.
> > Please see 
> > 6.2.5. Sequence of Device Error Signaling and Logging Operations
> > and figure 6-2 and 6-3.
> 
> My question was: the only difference appears to be between bridge and
> non-bridge devices: bridge has to do more stuff, but most code is
> common.  So this seems to be a very roundabout way to do this.
> Can't we just have a common function with an if (bridge) statement up front?
> If we ever only expect 2 implementations, I think a function pointer
> is overkill.

Not 2, but 3. root port, upstream or downstream and normal device.
So you want something like the following?
More than 2 is a good reason for me, I prefer function pointer.

switch (pcie_cap_get_type(dev))
    case PCI_EXP_TYPE_ROOT_PORT:
        pcie_aer_errmsg_root_port();
        break;
    case PCI_EXP_TYPE_DOWNSTREAM:
    case PCI_EXP_TYPE_UPSTREAM:
        pcie_aer_errmsg_vbridge();
        break;
    default:
       pcie_aer_errmsg_alldev();
       break;

-- 
yamahata