[libvirt] [PATCH v2 0/2] Add function for XML yes|no string handling

Changes since v1: - remove NULL check of string in virStringParseYesNo - remove error handling in virStringParseYesNo. - remove temporary variable. Shotaro Gotanda (2): util: add virStringParseYesNo() conf: Use virStringParseYesNo() src/conf/domain_conf.c | 30 +++++------------------------- src/conf/secret_conf.c | 12 ++---------- src/util/virstring.c | 23 +++++++++++++++++++++++ src/util/virstring.h | 3 +++ 4 files changed, 33 insertions(+), 35 deletions(-) -- 2.19.1

This function parse the string "yes" into bool true and "no" into false, and return 0. If the string is anything other than "yes|no", this function return -1. Signed-off-by: Shotaro Gotanda <g.sho1500@gmail.com> --- src/util/virstring.c | 23 +++++++++++++++++++++++ src/util/virstring.h | 3 +++ 2 files changed, 26 insertions(+) diff --git a/src/util/virstring.c b/src/util/virstring.c index 33f8191f45..945a8d0c84 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -1548,3 +1548,26 @@ virStringParsePort(const char *str, return 0; } + + +/** + * virStringParseYesNo: + * @str: "yes|no" to parse, and the value must not be NULL. + * @port: pointer to parse and convert "yes|no" into + * + * Parses a string "yes|no" and convert it into true|false. + * Returns 0 on success and -1 on error. + */ +int virStringParseYesNo(const char *str, bool *result) +{ + + if (STREQ(str, "yes")) { + *result = true; + } else if (STREQ(str, "no")) { + *result = false; + } else { + return -1; + } + + return 0; +} diff --git a/src/util/virstring.h b/src/util/virstring.h index 1e36ac459c..9b01e8568a 100644 --- a/src/util/virstring.h +++ b/src/util/virstring.h @@ -316,6 +316,9 @@ int virStringParsePort(const char *str, unsigned int *port) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; +int virStringParseYesNo(const char *str, + bool *result) + ATTRIBUTE_RETURN_CHECK; /** * VIR_AUTOSTRINGLIST: * -- 2.19.1

On Tue, Mar 12, 2019 at 18:56:22 +0900, Shotaro Gotanda wrote:
This function parse the string "yes" into bool true and "no" into false, and return 0. If the string is anything other than "yes|no", this function return -1.
Signed-off-by: Shotaro Gotanda <g.sho1500@gmail.com> --- src/util/virstring.c | 23 +++++++++++++++++++++++ src/util/virstring.h | 3 +++ 2 files changed, 26 insertions(+)
diff --git a/src/util/virstring.c b/src/util/virstring.c index 33f8191f45..945a8d0c84 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -1548,3 +1548,26 @@ virStringParsePort(const char *str,
return 0; } + + +/** + * virStringParseYesNo: + * @str: "yes|no" to parse, and the value must not be NULL. + * @port: pointer to parse and convert "yes|no" into + * + * Parses a string "yes|no" and convert it into true|false. + * Returns 0 on success and -1 on error. + */ +int virStringParseYesNo(const char *str, bool *result) +{ + + if (STREQ(str, "yes")) { + *result = true; + } else if (STREQ(str, "no")) { + *result = false; + } else { + return -1;
This does not conform to our coding style. Please make sure to read the guidelines and run syntax-check before posting.
+ } + + return 0; +} diff --git a/src/util/virstring.h b/src/util/virstring.h index 1e36ac459c..9b01e8568a 100644 --- a/src/util/virstring.h +++ b/src/util/virstring.h @@ -316,6 +316,9 @@ int virStringParsePort(const char *str, unsigned int *port) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
+int virStringParseYesNo(const char *str, + bool *result) + ATTRIBUTE_RETURN_CHECK; /** * VIR_AUTOSTRINGLIST: * -- 2.19.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

Thank you for your feedback. I overlooked my coding style violation, because I ran "make syntax-check" and it finished without any error. I suppose that you mean the curly bracket should be removed from if and else sentence because the body and condition itself occupy a single line. If there is any other coding style violation, could you tell me what it is? Regards, Shotaro 2019年3月12日(火) 19:33 Peter Krempa <pkrempa@redhat.com>:
On Tue, Mar 12, 2019 at 18:56:22 +0900, Shotaro Gotanda wrote:
This function parse the string "yes" into bool true and "no" into false, and return 0. If the string is anything other than "yes|no", this function return -1.
Signed-off-by: Shotaro Gotanda <g.sho1500@gmail.com> --- src/util/virstring.c | 23 +++++++++++++++++++++++ src/util/virstring.h | 3 +++ 2 files changed, 26 insertions(+)
diff --git a/src/util/virstring.c b/src/util/virstring.c index 33f8191f45..945a8d0c84 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -1548,3 +1548,26 @@ virStringParsePort(const char *str,
return 0; } + + +/** + * virStringParseYesNo: + * @str: "yes|no" to parse, and the value must not be NULL. + * @port: pointer to parse and convert "yes|no" into + * + * Parses a string "yes|no" and convert it into true|false. + * Returns 0 on success and -1 on error. + */ +int virStringParseYesNo(const char *str, bool *result) +{ + + if (STREQ(str, "yes")) { + *result = true; + } else if (STREQ(str, "no")) { + *result = false; + } else { + return -1;
This does not conform to our coding style. Please make sure to read the guidelines and run syntax-check before posting.
+ } + + return 0; +} diff --git a/src/util/virstring.h b/src/util/virstring.h index 1e36ac459c..9b01e8568a 100644 --- a/src/util/virstring.h +++ b/src/util/virstring.h @@ -316,6 +316,9 @@ int virStringParsePort(const char *str, unsigned int *port) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
+int virStringParseYesNo(const char *str, + bool *result) + ATTRIBUTE_RETURN_CHECK; /** * VIR_AUTOSTRINGLIST: * -- 2.19.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -----BEGIN PGP SIGNATURE-----
iQIzBAEBCAAdFiEEUn7DGLvflazX+2GwHGwCByjY1GoFAlyHiwgACgkQHGwCByjY 1GoDCw//ZpVdt+38W0A3Qdx8j4tKXRo4C3WLQEABRNIVXH+IF9esTYIQ7cmaH6rV p5bZQQILTC/lonDdxVBESYpOVUcU+VeZzMKLEqHJNTPdIyPvoFQTrpKD5Dm1VDcX 7TbBuMBvR6mkBqSu8MkreZ/UvyyIf16ATH/JWM0rTujJtZN5EIOYABkHsF+BBZrj tIC1E6d8nmkhw3U5KPDA94YzhptxKGsK+7wd9if9QDtd6TDoFsiJZcOyDYOBHOi/ fnbrBkckdbVSvI9IGYZMVdDqBBbjUK73v2dNQWWKffzzEVAXKPG7j0YB3rgVmDqE 6de3P+sbpgdg9jD5rHxSEaCEM5d6we3I5ajKK5brT3POmVbhrmmidlvWEKT2HTBB J/qs1tThRnaRUCcnYflhR+jl/7e6rkvA4FBv+1ycq4CCCDGujYfMJqjIIRRqel+j GveeFQlXf9UBiZbF6E5rCTqQsdPFlKVin0WgOQYXGj2gH7gJ+0e7XqZAoeMEtXqC X3DknMP31h+hFQIt/TftDZU+1ZloujYOWOvigQ7xxJgPqB2KNk1D37H6vbRoleUp o6xmtnOVc5PYbbTq1PGh7E62qeb6eOIvfe3OtpRDuN/zoHxJnnTIUVLKOf1H3Cqo y6c/4hFQ6oH52hIvyzNa2OOf+UM3S7/jmetj64YeC98jy7tmYTU= =bZTz -----END PGP SIGNATURE-----

[please do not top-post on technical lists] On Tue, Mar 12, 2019 at 23:22:00 +0900, Shotaro Gotanda wrote:
Thank you for your feedback. I overlooked my coding style violation, because I ran "make syntax-check" and it finished without any error.
I suppose that you mean the curly bracket should be removed from if and else sentence because the body and condition itself occupy a single line.
If there is any other coding style violation, could you tell me what it is?
also 4 spaces per indentation level

Thank you for your feedback and sorry for my bothering you. I’ll fix it, check if there is any other problems and send the patches again. Regards, Shotaro

this commit remove redundant common pattern in our XML parsing. Signed-off-by: Shotaro Gotanda <g.sho1500@gmail.com> --- src/conf/domain_conf.c | 30 +++++------------------------- src/conf/secret_conf.c | 12 ++---------- 2 files changed, 7 insertions(+), 35 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 995f87bcbe..3830926f3d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8412,11 +8412,7 @@ virSecurityLabelDefParseXML(xmlXPathContextPtr ctxt, p = virXMLPropStringLimit(ctxt->node, "relabel", VIR_SECURITY_LABEL_BUFLEN-1); if (p) { - if (STREQ(p, "yes")) { - seclabel->relabel = true; - } else if (STREQ(p, "no")) { - seclabel->relabel = false; - } else { + if (virStringParseYesNo(p, &seclabel->relabel) < 0) { virReportError(VIR_ERR_XML_ERROR, _("invalid security relabel value %s"), p); goto error; @@ -8646,11 +8642,7 @@ virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDefPtr **seclabels_rtn, relabel = virXMLPropString(list[i], "relabel"); if (relabel != NULL) { - if (STREQ(relabel, "yes")) { - seclabels[i]->relabel = true; - } else if (STREQ(relabel, "no")) { - seclabels[i]->relabel = false; - } else { + if (virStringParseYesNo(relabel, &seclabels[i]->relabel) < 0) { virReportError(VIR_ERR_XML_ERROR, _("invalid security relabel value %s"), relabel); @@ -13653,11 +13645,7 @@ virDomainGraphicsDefParseXMLSDL(virDomainGraphicsDefPtr def, ctxt->node = node; if (fullscreen != NULL) { - if (STREQ(fullscreen, "yes")) { - def->data.sdl.fullscreen = true; - } else if (STREQ(fullscreen, "no")) { - def->data.sdl.fullscreen = false; - } else { + if (virStringParseYesNo(fullscreen, &def->data.sdl.fullscreen) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unknown fullscreen value '%s'"), fullscreen); goto cleanup; @@ -13745,11 +13733,7 @@ virDomainGraphicsDefParseXMLDesktop(virDomainGraphicsDefPtr def, VIR_AUTOFREE(char *) fullscreen = virXMLPropString(node, "fullscreen"); if (fullscreen != NULL) { - if (STREQ(fullscreen, "yes")) { - def->data.desktop.fullscreen = true; - } else if (STREQ(fullscreen, "no")) { - def->data.desktop.fullscreen = false; - } else { + if (virStringParseYesNo(fullscreen, &def->data.desktop.fullscreen) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unknown fullscreen value '%s'"), fullscreen); return -1; @@ -15458,11 +15442,7 @@ virDomainRedirFilterUSBDevDefParseXML(xmlNodePtr node) allow = virXMLPropString(node, "allow"); if (allow) { - if (STREQ(allow, "yes")) { - def->allow = true; - } else if (STREQ(allow, "no")) { - def->allow = false; - } else { + if (virStringParseYesNo(allow, &def->allow) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid allow value, either 'yes' or 'no'")); goto error; diff --git a/src/conf/secret_conf.c b/src/conf/secret_conf.c index ca6cc194a2..5b85a7c0be 100644 --- a/src/conf/secret_conf.c +++ b/src/conf/secret_conf.c @@ -147,11 +147,7 @@ secretXMLParseNode(xmlDocPtr xml, xmlNodePtr root) prop = virXPathString("string(./@ephemeral)", ctxt); if (prop != NULL) { - if (STREQ(prop, "yes")) { - def->isephemeral = true; - } else if (STREQ(prop, "no")) { - def->isephemeral = false; - } else { + if (virStringParseYesNo(prop, &def->isephemeral) < 0) { virReportError(VIR_ERR_XML_ERROR, "%s", _("invalid value of 'ephemeral'")); goto cleanup; @@ -161,11 +157,7 @@ secretXMLParseNode(xmlDocPtr xml, xmlNodePtr root) prop = virXPathString("string(./@private)", ctxt); if (prop != NULL) { - if (STREQ(prop, "yes")) { - def->isprivate = true; - } else if (STREQ(prop, "no")) { - def->isprivate = false; - } else { + if (virStringParseYesNo(prop, &def->isprivate) < 0) { virReportError(VIR_ERR_XML_ERROR, "%s", _("invalid value of 'private'")); goto cleanup; -- 2.19.1
participants (2)
-
Peter Krempa
-
Shotaro Gotanda