[Qemu-devel] Re: [PATCH v3 06/13] pcie/aer: helper functions for pcie aer capability.

[Isaku Yamahata <yamahata@valinux.co.jp>]

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