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

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(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list