External/libvirt
1
0
Fork 0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2024-12-24 14:45:24 +00:00
Code Issues Packages Projects Releases Wiki Activity

util: xml: Fix confusing semantics of VIR_XML_PROP_OPTIONAL flag

Browse Source 
The new enum helpers use a set of flags to modify their behaviour, but
the declared set of flags is semantically confusing:

 typedef enum {
     VIR_XML_PROP_OPTIONAL = 0, /* Attribute may be absent */
     VIR_XML_PROP_REQUIRED = 1 << 0, /* Attribute may not be absent */

Since VIR_XML_PROP_OPTIONAL is declared as 0 any other flag shadows it
and makes it impossible to detect. The functions are not able to detect
a semantic nonsense of VIR_XML_PROP_OPTIONAL | VIR_XML_PROP_REQUIRED and
it's a perfectly valid statement for the compilers.

In general having two flags to do the same boolean don't make sense and
the implementation doesn't fix any shortcomings either.

To prevent mistakes, rename VIR_XML_PROP_OPTIONAL to VIR_XML_PROP_NONE,
so that there's always an enum value used with the calls but it doesn't
imply that the flag makes the property optional when the actual value is
0.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This commit is contained in:
Peter Krempa 2021-04-21 08:49:51 +02:00
parent 497c3ecd78
commit 45a61cbf68
4 changed files with 15 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/conf/cpu_conf.c
View File

@ -434,7 +434,7 @@ virCPUDefParseXML(xmlXPathContextPtr ctxt,
}
if (virXMLPropEnum(ctxt->node, "check", virCPUCheckTypeFromString,
VIR_XML_PROP_OPTIONAL, &def->check) < 0)
VIR_XML_PROP_NONE, &def->check) < 0)
return -1;
}

24
src/conf/domain_conf.c
View File

@ -9337,28 +9337,28 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt,
def->device = VIR_DOMAIN_DISK_DEVICE_DISK;
if (virXMLPropEnum(node, "device", virDomainDiskDeviceTypeFromString,
VIR_XML_PROP_OPTIONAL, &def->device) < 0)
VIR_XML_PROP_NONE, &def->device) < 0)
return NULL;
if (virXMLPropEnum(node, "model", virDomainDiskModelTypeFromString,
VIR_XML_PROP_OPTIONAL, &def->model) < 0)
VIR_XML_PROP_NONE, &def->model) < 0)
return NULL;
if (virXMLPropEnum(node, "snapshot", virDomainSnapshotLocationTypeFromString,
VIR_XML_PROP_OPTIONAL | VIR_XML_PROP_NONZERO, &def->snapshot) < 0)
VIR_XML_PROP_NONZERO, &def->snapshot) < 0)
return NULL;
if (virXMLPropTristateBool(node, "rawio", VIR_XML_PROP_OPTIONAL, &def->rawio) < 0)
if (virXMLPropTristateBool(node, "rawio", VIR_XML_PROP_NONE, &def->rawio) < 0)
return NULL;
if (virXMLPropEnum(node, "sgio", virDomainDeviceSGIOTypeFromString,
VIR_XML_PROP_OPTIONAL | VIR_XML_PROP_NONZERO, &def->sgio) < 0)
VIR_XML_PROP_NONZERO, &def->sgio) < 0)
return NULL;
if ((sourceNode = virXPathNode("./source", ctxt))) {
if (virXMLPropEnum(sourceNode, "startupPolicy",
virDomainStartupPolicyTypeFromString,
VIR_XML_PROP_OPTIONAL | VIR_XML_PROP_NONZERO,
VIR_XML_PROP_NONZERO,
&def->startupPolicy) < 0)
return NULL;
}
@ -9368,19 +9368,19 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt,
if (virXMLPropEnum(targetNode, "bus",
virDomainDiskBusTypeFromString,
VIR_XML_PROP_OPTIONAL | VIR_XML_PROP_NONZERO,
VIR_XML_PROP_NONZERO,
&def->bus) < 0)
return NULL;
if (virXMLPropEnum(targetNode, "tray", virDomainDiskTrayTypeFromString,
VIR_XML_PROP_OPTIONAL, &def->tray_status) < 0)
VIR_XML_PROP_NONE, &def->tray_status) < 0)
return NULL;
if (virXMLPropTristateSwitch(targetNode, "removable", VIR_XML_PROP_OPTIONAL,
if (virXMLPropTristateSwitch(targetNode, "removable", VIR_XML_PROP_NONE,
&def->removable) < 0)
return NULL;
if (virXMLPropUInt(targetNode, "rotation_rate", 10, VIR_XML_PROP_OPTIONAL,
if (virXMLPropUInt(targetNode, "rotation_rate", 10, VIR_XML_PROP_NONE,
&def->rotation_rate) < 0)
return NULL;
}
@ -9391,11 +9391,11 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt,
}
if ((blockioNode = virXPathNode("./blockio", ctxt))) {
if (virXMLPropUInt(blockioNode, "logical_block_size", 10, VIR_XML_PROP_OPTIONAL,
if (virXMLPropUInt(blockioNode, "logical_block_size", 10, VIR_XML_PROP_NONE,
&def->blockio.logical_block_size) < 0)
return NULL;
if (virXMLPropUInt(blockioNode, "physical_block_size", 10, VIR_XML_PROP_OPTIONAL,
if (virXMLPropUInt(blockioNode, "physical_block_size", 10, VIR_XML_PROP_NONE,
&def->blockio.physical_block_size) < 0)
return NULL;
}

2
src/conf/network_conf.c
View File

@ -1332,7 +1332,7 @@ virNetworkForwardNatDefParseXML(const char *networkName,
return -1;
}
if (virXMLPropTristateBool(node, "ipv6", VIR_XML_PROP_OPTIONAL,
if (virXMLPropTristateBool(node, "ipv6", VIR_XML_PROP_NONE,
&def->natIPv6) < 0)
return -1;

2
src/util/virxml.h
View File

@ -35,7 +35,7 @@ xmlXPathContextPtr virXMLXPathContextNew(xmlDocPtr xml)
typedef enum {
VIR_XML_PROP_OPTIONAL = 0, /* Attribute may be absent */
VIR_XML_PROP_NONE = 0,
VIR_XML_PROP_REQUIRED = 1 << 0, /* Attribute may not be absent */
VIR_XML_PROP_NONZERO = 1 << 1, /* Attribute may not be zero */
} virXMLPropFlags;