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

[Andrea Bolognani]

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