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@xxxxxxxxxx>
> ---
> src/conf/domain_conf.c | 20 +++++++++-----------
> 1 file changed, 9 insertions(+), 11 deletions(-)
>
Reviewed-by: John Ferlan <jferlan@xxxxxxxxxx>
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);
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
