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

[Marcel Apfelbaum <marcel@redhat.com>]

On 04/19/2017 03:05 PM, Cornelia Huck wrote:
> On Tue, 18 Apr 2017 22:42:07 -0300
> Eduardo Habkost <ehabkost@redhat.com> 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?

Thanks,
Marcel


>
> (the last one might be a bit long, though, especially as it takes so
> many arguments already)
>