Re: [PATCH 1 of 2] libxl: Move bdf parsing into libxlu

[George Dunlap <george.dunlap@eu.citrix.com>]

On 02/04/12 12:05, Ian Campbell wrote:
> On Mon, 2012-04-02 at 11:47 +0100, George Dunlap wrote:
>> # HG changeset patch
>> # User George Dunlap<george.dunlap@eu.citrix.com>
>> # Date 1333362574 -3600
>> # Node ID 5386937e6c5c9afaa8a3cd56d391dcc9e40d0596
>> # Parent  f744e82ea74075983de6d5b0ad0cf7ccacf999a2
>> libxl: Move bdf parsing into libxlu
>>
>> Config parsing functions do not properly belong in libxl.  Move them into
>> libxlu so that others can use them or not as they see fit.
>>
>> No functional changes.  One side-effect was making public a private libxl
>> utility function which just set the elements of a structure from the  function
>> arguments passed in.
>> [...]
>> diff -r f744e82ea740 -r 5386937e6c5c tools/libxl/libxl.h
>> --- a/tools/libxl/libxl.h       Wed Feb 29 16:30:34 2012 +0000
>> +++ b/tools/libxl/libxl.h       Mon Apr 02 11:29:34 2012 +0100
>> @@ -573,13 +573,10 @@ int libxl_device_pci_add(libxl_ctx *ctx,
>>   int libxl_device_pci_remove(libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev);
>>   int libxl_device_pci_destroy(libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev);
>>   libxl_device_pci *libxl_device_pci_list(libxl_ctx *ctx, uint32_t domid, int *num);
>> -
>> -/*
>> - * Parse a PCI BDF into a PCI device structure.
>> - */
>> -int libxl_device_pci_parse_bdf(libxl_ctx *ctx,
>> -                               libxl_device_pci *pcidev,
>> -                               const char *str);
>> +/* Just initialize the structure elements with the arguments provided. */
>> +int libxl_pci_dev_init(libxl_device_pci *pcidev, unsigned int domain,
>> +                       unsigned int bus, unsigned int dev,
>> +                       unsigned int func, unsigned int vdevfn);
> libxl_<type>_init has a particular meaning described further up in this
> header. Although you haven't actually used<type>  here so it doesn't
> conflict the general convention is to use the type name as a prefix.
>
> Does this function actually add all that much value? The users of it
> could either open code it or have a local version.
You know, I think I'll just make two local copies of the function.  I'll 
also give it a less misleading name.

  -George