[libvirt] [PATCH v3 01/12] conf: Add definitions for 'uid' and 'fid' PCI address attributes

[Andrea Bolognani]

On Tue, 2018-08-07 at 17:10 +0800, Yi Min Zhao wrote:
> +typedef struct _virZPCIDeviceAddress virZPCIDeviceAddress;
> +typedef virZPCIDeviceAddress *virZPCIDeviceAddressPtr;
> +struct _virZPCIDeviceAddress {
> +    unsigned int zpci_fid;
> +    unsigned int zpci_uid;
> +    bool fid_assigned;
> +    bool uid_assigned;
> +};

A couple of questions about the approach here, one of which I have
mentioned already and one of which I probably haven't (my bad):

  * do you really need to have separate booleans tracking whether
    or not either id has been assigned? Wouldn't the same approach
    as virPCIDeviceAddressIsEmpty() work, eg. consider the address
    absent if both are zero and present otherwise?

  * especially if you don't need the additional booleans, would it
    be preferable to just add the two ids to the existing struct
    instead of declaring a new one that you'll have to allocate
    and make sure it's not NULL before accessing it? Again, I seem
    to remember Laine feeling somewhat strongly about the topic.

Cosmetic issues:

  * uid should be listed before fid;

  * the zpci_ prefix is unnecessary if you have a separate struct
    that contains "ZPCI" in the name. But see above :)

-- 
Andrea Bolognani / Red Hat / Virtualization