[libvirt] [PATCH v2 0/3] use virStringParseYesNo helper
A function virStringParseYesNo was added to convert string 'yes' to true and 'no' to false, so use this helper to replace 'STREQ(.*, \"yes\")' and 'STREQ(.*, \"no\")' as it allows us to drop several repetitive if-then-else string->bool conversion blocks. v2->v1 p1: - ignore the return value of virStringParseYesNo. - update the commit message. [Michal Privoznik] p2: - add the Acked-by tag. p3: - pass return value of helper to rc directly. [Michal Privoznik] Mao Zhongyi (3): conf/domain_conf: use virStringParseYesNo helper conf/network_conf: use virStringParseYesNo helper qemu/qemu_migration_params: use virStringParseYesNo helper src/conf/domain_conf.c | 35 ++++++++++++-------------------- src/conf/network_conf.c | 4 +--- src/qemu/qemu_migration_params.c | 7 +------ 3 files changed, 15 insertions(+), 31 deletions(-) -- 2.17.1
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 to handle the conversion and reserve the original logic of not raise an error, so ignore the return value of helper. Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com> Signed-off-by: Zhang Shengju <zhangshengju@cmss.chinamobile.com> --- src/conf/domain_conf.c | 35 +++++++++++++---------------------- 1 file changed, 13 insertions(+), 22 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 10d6bf0eea..370e2840eb 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7600,10 +7600,8 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node, } } - if ((autoAddress = virXMLPropString(node, "autoAddress"))) { - if (STREQ(autoAddress, "yes")) - usbsrc->autoAddress = true; - } + if ((autoAddress = virXMLPropString(node, "autoAddress"))) + ignore_value(virStringParseYesNo(autoAddress, &usbsrc->autoAddress)); /* Product can validly be 0, so we need some extra help to determine * if it is uninitialized*/ @@ -8167,10 +8165,8 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, * element that might be (pure hostdev, or higher level device * (e.g. <interface>) with type='hostdev') */ - if ((managed = virXMLPropString(node, "managed")) != NULL) { - if (STREQ(managed, "yes")) - def->managed = true; - } + if ((managed = virXMLPropString(node, "managed")) != NULL) + ignore_value(virStringParseYesNo(managed, &def->managed)); sgio = virXMLPropString(node, "sgio"); rawio = virXMLPropString(node, "rawio"); @@ -13807,9 +13803,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); @@ -13939,13 +13933,10 @@ 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 { - def->data.vnc.autoport = false; - } + ignore_value(virStringParseYesNo(autoport, &def->data.vnc.autoport)); + + if (def->data.vnc.autoport && flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) + def->data.vnc.port = 0; } if (websocket) { @@ -13958,8 +13949,9 @@ virDomainGraphicsDefParseXMLVNC(virDomainGraphicsDefPtr def, } } - if (websocketGenerated && STREQ(websocketGenerated, "yes")) - def->data.vnc.websocketGenerated = true; + if (websocketGenerated) + ignore_value(virStringParseYesNo(websocketGenerated, + &def->data.vnc.websocketGenerated)); if (sharePolicy) { int policy = @@ -15479,8 +15471,7 @@ virDomainVideoDefParseXML(virDomainXMLOptionPtr xmlopt, heads = virXMLPropString(cur, "heads"); if ((primary = virXMLPropString(cur, "primary")) != NULL) { - if (STREQ(primary, "yes")) - def->primary = true; + ignore_value(virStringParseYesNo(primary, &def->primary)); VIR_FREE(primary); } -- 2.17.1
A function virStringParseYesNo was added to convert string 'yes' to true and 'no' to false, so use this helper to replace 'STREQ(.*, \"yes\")' and 'STREQ(.*, \"no\")' as it allows us to drop several repetitive if-then-else string->bool conversion blocks. Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com> Signed-off-by: Zhang Shengju <zhangshengju@cmss.chinamobile.com> Acked-by: Michal Privoznik <mprivozn@redhat.com> --- src/conf/network_conf.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 75ec5ccc27..9954c3d25f 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -1682,9 +1682,7 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt, */ ipv6nogwStr = virXPathString("string(./@ipv6)", ctxt); if (ipv6nogwStr) { - if (STREQ(ipv6nogwStr, "yes")) { - def->ipv6nogw = true; - } else if (STRNEQ(ipv6nogwStr, "no")) { + if (virStringParseYesNo(ipv6nogwStr, &def->ipv6nogw) < 0) { virReportError(VIR_ERR_XML_ERROR, _("Invalid ipv6 setting '%s' in network '%s'"), ipv6nogwStr, def->name); -- 2.17.1
A function virStringParseYesNo was added to convert string 'yes' to true and 'no' to false, so use this helper to replace 'STREQ(.*, \"yes\")' and 'STREQ(.*, \"no\")' as it allows us to drop several repetitive if-then-else string->bool conversion blocks. Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com> Signed-off-by: Zhang Shengju <zhangshengju@cmss.chinamobile.com> --- src/qemu/qemu_migration_params.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c index 85fa8f8de5..f5bc8596f4 100644 --- a/src/qemu/qemu_migration_params.c +++ b/src/qemu/qemu_migration_params.c @@ -1326,12 +1326,7 @@ qemuMigrationParamsParse(xmlXPathContextPtr ctxt, break; case QEMU_MIGRATION_PARAM_TYPE_BOOL: - if (STREQ(value, "yes")) - pv->value.b = true; - else if (STREQ(value, "no")) - pv->value.b = false; - else - rc = -1; + rc = virStringParseYesNo(value, &pv->value.b); break; case QEMU_MIGRATION_PARAM_TYPE_STRING: -- 2.17.1
On 10/16/19 11:19 PM, Mao Zhongyi wrote:
A function virStringParseYesNo was added to convert string 'yes' to true and 'no' to false, so use this helper to replace 'STREQ(.*, \"yes\")' and 'STREQ(.*, \"no\")' as it allows us to drop several repetitive if-then-else string->bool conversion blocks.
v2->v1
p1: - ignore the return value of virStringParseYesNo. - update the commit message. [Michal Privoznik]
p2: - add the Acked-by tag.
p3: - pass return value of helper to rc directly. [Michal Privoznik]
Mao Zhongyi (3): conf/domain_conf: use virStringParseYesNo helper conf/network_conf: use virStringParseYesNo helper qemu/qemu_migration_params: use virStringParseYesNo helper
src/conf/domain_conf.c | 35 ++++++++++++-------------------- src/conf/network_conf.c | 4 +--- src/qemu/qemu_migration_params.c | 7 +------ 3 files changed, 15 insertions(+), 31 deletions(-)
Reviewed-by: Cole Robinson <crobinso@redhat.com> And pushed Thanks, Cole
participants (2)
-
Cole Robinson -
Mao Zhongyi