On 10/16/19 4:39 AM, Mao Zhongyi wrote:
This helper performs a conversion from a "yes|no" string
to a corresponding boolean, and several conversions were
already done, but there are still some omissions.
For most of the remaining usages in domain_conf.c only
"yes" is explicitly checked for. This means all other
values are implicitly handled as 'false'. In this case,
use virStringParseYesNo, but if it returns -1, don't
raise an error but set the bool value to false.
Cc: crobinso@xxxxxxxxxx
Cc: berrange@xxxxxxxxxx
Cc: abologna@xxxxxxxxxx
Cc: laine@xxxxxxxxx
Cc: phrdina@xxxxxxxxxx
Cc: mkletzan@xxxxxxxxxx
Cc: g.sho1500@xxxxxxxxx
Signed-off-by: Mao Zhongyi <maozhongyi@xxxxxxxxxxxxxxxxxxxx>
Signed-off-by: Zhang Shengju <zhangshengju@xxxxxxxxxxxxxxxxxxxx>
---
src/conf/domain_conf.c | 30 ++++++++++++++----------------
1 file changed, 14 insertions(+), 16 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 10d6bf0eea..7420658726 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -7601,8 +7601,8 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node,
}
if ((autoAddress = virXMLPropString(node, "autoAddress"))) {
- if (STREQ(autoAddress, "yes"))
- usbsrc->autoAddress = true;
+ if (virStringParseYesNo(autoAddress, &usbsrc->autoAddress) < 0)
+ usbsrc->autoAddress = false;
I think we should either report a proper error here or don't touch this
line (because this attribute is generated by libvirt itself, it doesn't
come from an user).
}
/* Product can validly be 0, so we need some extra help to determine
@@ -8168,8 +8168,8 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node,
* (e.g. <interface>) with type='hostdev')
*/
if ((managed = virXMLPropString(node, "managed")) != NULL) {
- if (STREQ(managed, "yes"))
- def->managed = true;
+ if (virStringParseYesNo(managed, &def->managed) < 0)
+ def->managed = false;
Here we need to report a proper error because "managed" attribute comes
from user, therefore risk of an user providing invalid value is high
compared to the first hunk. But if we want to be consistent we should
report error in both cases.
}
sgio = virXMLPropString(node, "sgio");
@@ -13807,9 +13807,7 @@ virDomainGraphicsListenDefParseXML(virDomainGraphicsListenDefPtr def,
if (autoGenerated &&
flags & VIR_DOMAIN_DEF_PARSE_STATUS) {
- if (STREQ(autoGenerated, "yes")) {
- def->autoGenerated = true;
- } else if (STRNEQ(autoGenerated, "no")) {
+ if (virStringParseYesNo(autoGenerated, &def->autoGenerated) < 0) {
virReportError(VIR_ERR_XML_ERROR,
_("Invalid autoGenerated value: %s"),
autoGenerated);
This one is correct.
@@ -13939,12 +13937,11 @@ virDomainGraphicsDefParseXMLVNC(virDomainGraphicsDefPtr def,
}
if (autoport) {
- if (STREQ(autoport, "yes")) {
- if (flags & VIR_DOMAIN_DEF_PARSE_INACTIVE)
- def->data.vnc.port = 0;
- def->data.vnc.autoport = true;
- } else {
+ if (virStringParseYesNo(autoport, &def->data.vnc.autoport) < 0) {
def->data.vnc.autoport = false;
+ } else {
+ if (def->data.vnc.autoport && flags & VIR_DOMAIN_DEF_PARSE_INACTIVE)
+ def->data.vnc.port = 0;
This doesn't need to go under else branch really.
}
}
@@ -13958,8 +13955,9 @@ virDomainGraphicsDefParseXMLVNC(virDomainGraphicsDefPtr def,
}
}
- if (websocketGenerated && STREQ(websocketGenerated, "yes"))
- def->data.vnc.websocketGenerated = true;
+ if (websocketGenerated &&
+ virStringParseYesNo(websocketGenerated, &def->data.vnc.websocketGenerated))
Ooops, you've missed '< 0' comparison.
+ def->data.vnc.websocketGenerated = false;
if (sharePolicy) {
int policy =
@@ -15479,8 +15477,8 @@ virDomainVideoDefParseXML(virDomainXMLOptionPtr xmlopt,
heads = virXMLPropString(cur, "heads");
if ((primary = virXMLPropString(cur, "primary")) != NULL) {
- if (STREQ(primary, "yes"))
- def->primary = true;
+ if (virStringParseYesNo(primary, &def->primary) < 0)
+ def->primary = false;
Again, I'd rather see an error reported here.
VIR_FREE(primary);
}
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
