[libvirt] [PATCH] conf: Fix initialization value of 'multi' in PCI address

[Xian Han Yu]

On 8/10/2016 11:34 PM, Andrea Bolognani wrote:
> On Wed, 2016-08-10 at 17:16 +0200, Boris Fiuczynski wrote:
>>>>>        for (i = 0; i < nAddrNodes; i++) {
>>>>> -        virPCIDeviceAddress addr = { 0, 0, 0, 0, 0 };
>>>>> +        virPCIDeviceAddress addr = { 0, 0, 0, 0, false };
>>>>   
>>>> Honestly, I have no idea what preferences we have for such
>>>> initializations, but I for one prefer initialization to '{0}' which
>>>> guarantees everything to be zeroed anyway.  And will be readable the
>>>> same way even when we change the structure.  Would that work for you as
>>>> well?
>>>   
>>> I think it should either be 0 (as the structure member is
>>> defined as int) or VIR_TRISTATE_SWITCH_ABSENT (as it is used
>>> as virTristateSwitch, according to the comment and other bits
>>> of code). false definitely seems out of place.
>>   
>> Actually this fix was about aligning three code occurrences.
>> These three initialisations can be found here:
>>   
>> qemu/qemu_domain_address.c
>> 1099:            virPCIDeviceAddress addr = { 0, 0, 0, 0, false };
>>   
>> conf/node_device_conf.c
>> 1166:        virPCIDeviceAddress addr = { 0, 0, 0, 0, 0 };
>>   
>> conf/domain_addr.c
>> 572:    virPCIDeviceAddress a = { 0, 0, 0, 0, false };
>>   
>> Setting the VIR_TRISTATE_SWITCH_ABSENT make sense from the data type
>> point of view. Looking at it from the code readability point of view you
>> would have to know that the default of the multifunction is Off and with
>> that in mind it made more sense setting it to false.
> The default is not OFF, though, it's ABSENT :)
>
> In fact, as far as I can tell, OFF isn't ever used explicitly
> either for assignment or comparison. And false is plain wrong
> from a datatype point of view.
>
> -- 
> Andrea Bolognani / Red Hat / Virtualization
>
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

How about we change all three occurrences as boris list above into 
VIR_TRISTATE_SWITCH_ABSENT.

Xian Han

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20160811/3e174440/attachment-0001.htm>