[libvirt] [PATCH v3 01/12] conf: Add definitions for 'uid' and 'fid' PCI address attributes
Yi Min Zhao
zyimin at linux.ibm.com
Mon Aug 20 08:19:48 UTC 2018
在 2018/8/16 下午10:38, 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?
It's OK for uid. But for fid, zero is a valid value, so we need a bool
to track its assignment.
If we add a boolean for fid, why not also add another one for uid to
keep consistency?
This also makes code easy to read and obvious. It doesn't waste much
memory space.
>
> * 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.
For other platforms, is it OK to waste this unused memory (of course,
it's little) ?
>
> 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 :)
>
More information about the libvir-list
mailing list