On 3/18/21 9:00 AM, 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.
>
> Patch #11 (virDomainChrSourceReconnectDefParseXML) is a good example of what
> this refactoring is about.
>
> Tim Wiederhake (38):
> virStorageAdapterFCHost: Fix comment
> virxml: Add virXMLPropYesNo
> virxml: Add virXMLPropOnOff
> domain_conf: Use virXMLProp(OnOff|YesNo) in
> virDomainKeyWrapCipherDefParseXML
> domain_conf: Use virXMLProp(OnOff|YesNo) in
> virDomainVirtioOptionsParseXML
> domain_conf: Use virXMLProp(OnOff|YesNo) in
> virDomainDeviceInfoParseXML
> domain_conf: Use virXMLProp(OnOff|YesNo) in
> virDomainDiskSourceNetworkParse
> domain_conf: Use virXMLProp(OnOff|YesNo) in
> virDomainDiskSourceNVMeParse
> domain_conf: Use virXMLProp(OnOff|YesNo) in
> virDomainDiskDefDriverParseXML
> domain_conf: Use virXMLProp(OnOff|YesNo) in
> virDomainActualNetDefParseXML
> domain_conf: Use virXMLProp(OnOff|YesNo) in
> virDomainChrSourceReconnectDefParseXML
> domain_conf: Use virXMLProp(OnOff|YesNo) in virDomainNetDefParseXML
> domain_conf: Use virXMLProp(OnOff|YesNo) in
> virDomainChrSourceDefParseTCP
> domain_conf: Use virXMLProp(OnOff|YesNo) in
> virDomainChrSourceDefParseFile
> domain_conf: Use virXMLProp(OnOff|YesNo) in
> virDomainChrSourceDefParseLog
> domain_conf: Use virXMLProp(OnOff|YesNo) in
> virDomainGraphicsDefParseXMLVNC
> domain_conf: Use virXMLProp(OnOff|YesNo) in
> virDomainGraphicsDefParseXMLSDL
> domain_conf: Use virXMLProp(OnOff|YesNo) in
> virDomainGraphicsDefParseXMLSpice
> domain_conf: Use virXMLProp(OnOff|YesNo) in virDomainAudioCommonParse
> domain_conf: Use virXMLProp(OnOff|YesNo) in virDomainAudioJackParse
> domain_conf: Use virXMLProp(OnOff|YesNo) in virDomainAudioOSSParse
> domain_conf: Use virXMLProp(OnOff|YesNo) in virDomainAudioDefParseXML
> domain_conf: Use virXMLProp(OnOff|YesNo) in
> virDomainMemballoonDefParseXML
> domain_conf: Use virXMLProp(OnOff|YesNo) in virDomainShmemDefParseXML
> domain_conf: Use virXMLProp(OnOff|YesNo) in
> virDomainPerfEventDefParseXML
> domain_conf: Use virXMLProp(OnOff|YesNo) in virDomainMemoryDefParseXML
> domain_conf: Use virXMLProp(OnOff|YesNo) in virDomainIOMMUDefParseXML
> domain_conf: Use virXMLProp(OnOff|YesNo) in virDomainVsockDefParseXML
> domain_conf: Use virXMLProp(OnOff|YesNo) in virDomainFeaturesDefParse
> domain_conf: Use virXMLProp(OnOff|YesNo) in virDomainLoaderDefParseXML
> domain_conf: Use virXMLProp(OnOff|YesNo) in virDomainVcpuParse
> backup_conf: Use virXMLProp(OnOff|YesNo) in
> virDomainBackupDiskDefParseXML
> backup_conf: Use virXMLProp(OnOff|YesNo) in virDomainBackupDefParse
> device_conf: Use virXMLProp(OnOff|YesNo) in
> virPCIDeviceAddressParseXML
> network_conf: Use virXMLProp(OnOff|YesNo) in
> virNetworkForwardNatDefParseXML
> numa_conf: Use virXMLProp(OnOff|YesNo) in virDomainNumaDefParseXML
> storage_adapter_conf: Use virXMLProp(OnOff|YesNo) in
> virStorageAdapterParseXMLFCHost
> storage_conf: Use virXMLProp(OnOff|YesNo) in
> virStoragePoolDefParseSource
>
> src/conf/backup_conf.c | 32 +-
> src/conf/device_conf.c | 10 +-
> src/conf/domain_conf.c | 791 +++++++++-----------------------
> src/conf/network_conf.c | 15 +-
> src/conf/numa_conf.c | 13 +-
> src/conf/storage_adapter_conf.c | 17 +-
> src/conf/storage_adapter_conf.h | 2 +-
> src/conf/storage_conf.c | 16 +-
> src/util/virxml.c | 74 +++
> src/util/virxml.h | 7 +
> 10 files changed, 314 insertions(+), 663 deletions(-)
>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Nice cleanup!
And if you agree with my suggestions in 01-02/38 (suggestion from 02
affects also 03), I can squash the changes in and push.
Michal