On 1/30/19 12:40 PM, Daniel P. Berrangé wrote:
'val' is initialized from virDomainCapsFeatureTypeFromString and a few lines earlier there was already a check for 'val < 0'.
The 'val >= 0' is thus always true. The enum conversion similarly ensures that the val will be less than VIR_DOMAIN_CAPS_FEATURE_LAST, so "val < VIR_DOMAIN_CAPS_FEATURE_LAST' is thus always true too.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/conf/domain_conf.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> However, do see below for a change that either could be made now or as a separate patch afterwards with an implied R-by as well. John
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9409d93c23..f2ef53a9a3 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -20524,18 +20524,16 @@ virDomainDefParseXML(xmlDocPtr xml, goto error; }
- if (val >= 0 && val < VIR_DOMAIN_CAPS_FEATURE_LAST) { - if ((tmp = virXMLPropString(nodes[i], "state"))) { - if ((def->caps_features[val] = virTristateSwitchTypeFromString(tmp)) == -1) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown state attribute '%s' of feature capability '%s'"), - tmp, virDomainFeatureTypeToString(val)); - goto error; - } - VIR_FREE(tmp); - } else { - def->caps_features[val] = VIR_TRISTATE_SWITCH_ON; + if ((tmp = virXMLPropString(nodes[i], "state"))) { + if ((def->caps_features[val] = virTristateSwitchTypeFromString(tmp)) == -1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown state attribute '%s' of feature capability '%s'"), + tmp, virDomainFeatureTypeToString(val)); ^^^^ This caught my eye...
Since @val was sourced from virDomainCapsFeatureTypeFromString so one would think it should use virDomainCapsFeatureTypeToString(val) at the very least. Or use 'nodes[i]->name' directly. As a test I modified tests/domainschemadata/domain-caps-features.xml to set state to "foo" and then creating the lxc guest would result in : error: unsupported configuration: unknown state attribute 'foo' of feature capability 'hpt' fixing to use DomainCapsFeature got: error: unsupported configuration: unknown state attribute 'foo' of feature capability 'mknod' I checked other virDomainFeatureTypeToString callers and they all seemed OK - it was just this one.
+ goto error; } + VIR_FREE(tmp); + } else { + def->caps_features[val] = VIR_TRISTATE_SWITCH_ON; } } VIR_FREE(nodes);