Re: [Qemu-devel] [RFC v2 3/6] pci: Rename and change signatures of pci_bus_new() & related functions

[Eduardo Habkost]

On Wed, Apr 19, 2017 at 09:41:19PM +0300, Marcel Apfelbaum wrote:
> On 04/19/2017 03:05 PM, Cornelia Huck wrote:
> > On Tue, 18 Apr 2017 22:42:07 -0300
> > Eduardo Habkost <address@hidden> wrote:
> > 
> > > On Wed, Apr 19, 2017 at 10:29:26AM +1000, David Gibson wrote:
> > > > On Tue, Apr 18, 2017 at 07:17:21PM -0300, Eduardo Habkost wrote:
> > > > > pci_bus_new*() and pci_register_bus() work only when the 'parent'
> > > > > argument is a PCI_HOST_BRIDGE object. Rename them to reflect that they
> > > > > are meant to initialize a bus that's in a PCI host bridge.
> > > > > 
> > > > > The new function names are:
> > > > > * pci_host_bus_init() (replacing pci_bus_new())
> > > > > * pci_host_bus_init_inplace() (replacing pci_bus_new_inplace())
> > > > > * pci_host_bus_init_irqs() (replacing pci_register_bus())
> > > > 
> > > > I like the idea, but I'm not terribly convinced by these names.
> > > > Aren't functions which allocate objects usually called whatever_new()
> > > > rather than whatever_init()?  And pci_register_bus() appears to do
> > > > more than just initialize irqs.
> > > 
> > > I agree the names aren't terribly clear. This is what they are
> > > supposed to mean:
> > > 
> > > * pci_host_bus_init(phb) initializes phb->bus.
> > > * pci_host_bus_init(phb) initializes phb->bus using an
> > >   already-allocated object.
> > > * pci_host_bus_init_irqs() does the same as pci_host_bus_init(),
> > >   but also calls pci_bus_irqs().
> > > 
> > > I plan to submit API documentation comments later. I am open to
> > > alternative name suggestions.
> > > 
> > 
> > pci_host_bus_init_irqs() sounds as if it would only init irqs. What
> 
> Right! This is what I thought too.
> 
> > about:
> > 
> > pci_host_bus_new()
> > pci_host_bus_new_inplace()
> > pci_host_bus_new_with_irqs()
> 
> I like the names, but a following patch (5/6) modifies
> the above functions to return void and create the bus
> as a side effect.
> So now we have a pci_host_bus_new that returns nothing?

I plan to change the approach, and not get rid of pci_bus_new().
I will probably just move the PCI_HOST_BRIDGE-specific code to a
separate function (probably I will name it pci_host_init_bus()),
that will call pci_bus_new(), append the bridge to
pci_host_bridges, and set PCIHostState::bus.

After that, we can change pci_bridge_initfn() to use
pci_bus_new() instead of reimplementing it.

-- 
Eduardo