On 3/19/21 4:56 PM, Tim Wiederhake wrote:
> This series replaces some recurring boilerplate code in src/conf/ regarding
> the extraction of a virTristate(Switch|Bool) XML attribute.
>
> The boilerplate code looks roughly like this,
>
> g_autofree char *str = NULL;
> if (str = virXMLPropString(node, ...)) {
> int val;
> if ((val = virTristateBoolTypeFromString(str)) <= 0) {
> virReportError(...)
> return -1;
> }
> def->... = val;
> }
>
> with some variations regarding how `str` is free'd in case of later re-use,
> the exact error message for invalid values, whether or not
> `VIR_TRISTATE_(SWITCH|BOOL)_ABSENT` is explicitly checked for (`val < 0` vs.
> `val <= 0`), whether an intermediate variable is used or the value is assigned
> directly, and in some cases the conditions in the two if-blocks are merged.
>
> As a side effect, this makes the error messages for invalid values in these
> attributes much more consistent and catches some instances where e.g.
> `<foo bar="default"/>` would incorrectly be accepted.
>
> V1: https://listman.redhat.com/archives/libvir-list/2021-March/msg00853.html
> V2: https://listman.redhat.com/archives/libvir-list/2021-March/msg00994.html
>
> Changes since V2:
> * Fixed the -Wtautological-unsigned-enum-zero-compare issues in the first
> part of the series. These issues were solved later in the series, but
> this change makes it easier to bisect in the future.
>
> I was thinking about only re-sending the first couple of patches, but the
> latter part would have endet up with quite a few conflicts, so I am sending
> it in its entirety, again. Sorry for the spam!
>
> Cheers,
> Tim
Alternatively, rewrite to virXMLPropTristateXXX() could have gone in the
first, followed by change of struct members to virTristateXXX which
would have produced smalled diff. But I guess it doesn't matter.
>
> Tim Wiederhake (51):
> conf: Use virTristateXXX in virStorageSource
> conf: Use virTristateXXX in virStorageSourceNVMeDef
> conf: Use virTristateXXX in virDomainDeviceInfo
> conf: Use virTristateXXX in virDomainDiskDef
> conf: Use virTristateXXX in virDomainActualNetDef
> conf: Use virTristateXXX in virDomainNetDef
> conf: Use virTristateXXX in virDomainChrSourceDef
> conf: Use virTristateXXX in virDomainGraphicsDef
> conf: Use virTristateXXX in virDomainMemballoonDef
> conf: Use virTristateXXX in virDomainLoaderDef
> conf: Use virTristateXXX in virDomainDef
> conf: Use virTristateXXX in virStorageAdapterFCHost
> conf: Use virTristateXXX in virStoragePoolSourceDevice
> conf: Use virTristateXXX in virPCIDeviceAddress
> virxml: Add virXMLPropTristateBool
> virxml: Add virXMLPropTristateSwitch
> domain_conf: Use virXMLPropTristateXXX in
> virDomainKeyWrapCipherDefParseXML
> domain_conf: Use virXMLPropTristateXXX in
> virDomainVirtioOptionsParseXML
> domain_conf: Use virXMLPropTristateXXX in virDomainDeviceInfoParseXML
> domain_conf: Use virXMLPropTristateXXX in
> virDomainDiskSourceNetworkParse
> domain_conf: Use virXMLPropTristateXXX in virDomainDiskSourceNVMeParse
> domain_conf: Use virXMLPropTristateXXX in
> virDomainDiskDefDriverParseXML
> domain_conf: Use virXMLPropTristateXXX in
> virDomainActualNetDefParseXML
> domain_conf: Use virXMLPropTristateXXX in
> virDomainChrSourceReconnectDefParseXML
> domain_conf: Use virXMLPropTristateXXX in virDomainNetDefParseXML
> domain_conf: Use virXMLPropTristateXXX in
> virDomainChrSourceDefParseTCP
> domain_conf: Use virXMLPropTristateXXX in
> virDomainChrSourceDefParseFile
> domain_conf: Use virXMLPropTristateXXX in
> virDomainChrSourceDefParseLog
> domain_conf: Use virXMLPropTristateXXX in
> virDomainGraphicsDefParseXMLVNC
> domain_conf: Use virXMLPropTristateXXX in
> virDomainGraphicsDefParseXMLSDL
> domain_conf: Use virXMLPropTristateXXX in
> virDomainGraphicsDefParseXMLSpice
> domain_conf: Use virXMLPropTristateXXX in virDomainAudioCommonParse
> domain_conf: Use virXMLPropTristateXXX in virDomainAudioJackParse
> domain_conf: Use virXMLPropTristateXXX in virDomainAudioOSSParse
> domain_conf: Use virXMLPropTristateXXX in virDomainAudioDefParseXML
> domain_conf: Use virXMLPropTristateXXX in
> virDomainMemballoonDefParseXML
> domain_conf: Use virXMLPropTristateXXX in virDomainShmemDefParseXML
> domain_conf: Use virXMLPropTristateXXX in
> virDomainPerfEventDefParseXML
> domain_conf: Use virXMLPropTristateXXX in virDomainMemoryDefParseXML
> domain_conf: Use virXMLPropTristateXXX in virDomainIOMMUDefParseXML
> domain_conf: Use virXMLPropTristateXXX in virDomainVsockDefParseXML
> domain_conf: Use virXMLPropTristateXXX in virDomainFeaturesDefParse
> domain_conf: Use virXMLPropTristateXXX in virDomainLoaderDefParseXML
> domain_conf: Use virXMLPropTristateXXX in virDomainVcpuParse
> backup_conf: Use virXMLPropTristateXXX in
> virDomainBackupDiskDefParseXML
> backup_conf: Use virXMLPropTristateXXX in virDomainBackupDefParse
> device_conf: Use virXMLPropTristateXXX in virPCIDeviceAddressParseXML
> network_conf: Use virXMLPropTristateXXX in
> virNetworkForwardNatDefParseXML
> numa_conf: Use virXMLPropTristateXXX in virDomainNumaDefParseXML
> storage_adapter_conf: Use virXMLPropTristateXXX in
> virStorageAdapterParseXMLFCHost
> storage_conf: Use virXMLPropTristateXXX in
> virStoragePoolDefParseSource
>
> src/conf/backup_conf.c | 32 +-
> src/conf/device_conf.c | 8 +-
> src/conf/device_conf.h | 4 +-
> src/conf/domain_conf.c | 794 +++++++-------------------------
> src/conf/domain_conf.h | 28 +-
> src/conf/network_conf.c | 15 +-
> src/conf/numa_conf.c | 14 +-
> src/conf/storage_adapter_conf.c | 14 +-
> src/conf/storage_adapter_conf.h | 2 +-
> src/conf/storage_conf.c | 17 +-
> src/conf/storage_conf.h | 2 +-
> src/conf/storage_source_conf.h | 4 +-
> src/libvirt_private.syms | 2 +
> src/qemu/qemu_command.c | 3 +-
> src/qemu/qemu_hotplug.c | 2 +-
> src/util/virpci.h | 2 +-
> src/util/virxml.c | 82 ++++
> src/util/virxml.h | 9 +
> 18 files changed, 301 insertions(+), 733 deletions(-)
>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Will push at the end of day, to give others chance to object.
Michal