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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 08/09/2016 11:25 AM, Andrea Bolognani wrote:
On Tue, 2016-08-09 at 11:19 +0200, Martin Kletzander 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.

--
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

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.

--
Mit freundlichen Grüßen/Kind regards
   Boris Fiuczynski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martina Köderitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]