[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v2 2/4] PCI: move pdev_list field to common structure



>>> On 04.06.19 at 15:05, <julien.grall@xxxxxxx> wrote:
> Hi Jan,
> 
> On 6/4/19 2:03 PM, Jan Beulich wrote:
>>>>> On 04.06.19 at 14:55, <julien.grall@xxxxxxx> wrote:
>>> On 6/4/19 1:42 PM, Jan Beulich wrote:
>>>> --- a/xen/include/xen/pci.h
>>>> +++ b/xen/include/xen/pci.h
>>>> @@ -121,7 +121,9 @@ struct pci_dev {
>>>>    };
>>>>    
>>>>    #define for_each_pdev(domain, pdev) \
>>>> -    list_for_each_entry(pdev, &(domain->arch.pdev_list), domain_list)
>>>> +    list_for_each_entry(pdev, &(domain)->pdev_list, domain_list)
>>>> +
>>>> +#define has_arch_pdevs(d) (!list_empty(&(d)->pdev_list))
>>>
>>> This feels a bit strange to keep "arch" in the macro name when the code
>>> is now generic. How about "domain_has_pdevs"?
>> 
>> Indeed I did notice this oddity too, but if such an adjustment is
>> to be made then imo not in this patch. After all in turn I'd need
>> to change all use sites.
> 
> It feels to me they are kind of tied together because has_arch_pdevs is 
> an accessor of the field.

In a way they are. But the name of the macro hasn't become
"wrong" by the change here. That renaming you ask for could
also have been done a year ago, if so wanted.

> But I am happy to see this in a separate patches.

FAOD - I didn't promise anything.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.