[libvirt] [PATCH v3 0/2] Add function for XML yes|no string handling
Changes since v2: - adjust the code to the coding guideline 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 | 21 +++++++++++++++++++++ src/util/virstring.h | 3 +++ 4 files changed, 31 insertions(+), 35 deletions(-) -- 2.19.1
This function parse the string "yes" into bool true and "no" into false, and returns 0. If the string is anything other than "yes|no", this function returns -1. Signed-off-by: Shotaro Gotanda <g.sho1500@gmail.com> --- src/util/virstring.c | 21 +++++++++++++++++++++ src/util/virstring.h | 3 +++ 2 files changed, 24 insertions(+) diff --git a/src/util/virstring.c b/src/util/virstring.c index 33f8191f45..3bbe36ea25 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -1548,3 +1548,24 @@ 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 Wed, Mar 13, 2019 at 03:30:10PM +0900, Shotaro Gotanda wrote:
This function parse the string "yes" into bool true and "no" into false, and returns 0. If the string is anything other than "yes|no", this function returns -1.
Signed-off-by: Shotaro Gotanda <g.sho1500@gmail.com> --- src/util/virstring.c | 21 +++++++++++++++++++++ src/util/virstring.h | 3 +++ 2 files changed, 24 insertions(+)
diff --git a/src/util/virstring.c b/src/util/virstring.c index 33f8191f45..3bbe36ea25 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -1548,3 +1548,24 @@ 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
^This one looks like a copy-paste from virStringParsePort ;)
+ * + * Parses a string "yes|no" and convert it into true|false.
...converts it into a boolean.
+ * 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)
^misaligned The function should also be added to src/libvirt_private.syms I tweaked the commit message a tiny bit and I'll perform the adjustments I noted before pushing by squashing the following: diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 960a97cf1d..92a86943b1 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2988,6 +2988,7 @@ virStringListRemove; virStringMatch; virStringMatchesNameSuffix; virStringParsePort; +virStringParseYesNo; virStringReplace; virStringSearch; virStringSortCompare; diff --git a/src/util/virstring.c b/src/util/virstring.c index d9d6c0a33e..95bd7d225e 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -1552,10 +1552,11 @@ virStringParsePort(const char *str, /** * virStringParseYesNo: - * @str: "yes|no" to parse, and the value must not be NULL. - * @port: pointer to parse and convert "yes|no" into + * @str: "yes|no" to parse, must not be NULL. + * @result: pointer to the boolean result of @str conversion + * + * Parses a "yes|no" string and converts it into a boolean. * - * 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) diff --git a/src/util/virstring.h b/src/util/virstring.h index 9b01e8568a..b0fedf96a3 100644 --- a/src/util/virstring.h +++ b/src/util/virstring.h @@ -317,7 +317,7 @@ int virStringParsePort(const char *str, ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; int virStringParseYesNo(const char *str, - bool *result) + bool *result) ATTRIBUTE_RETURN_CHECK; /** Reviewed-by: Erik Skultety <eskultet@redhat.com>
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
On Wed, Mar 13, 2019 at 03:30:11PM +0900, Shotaro Gotanda wrote:
this commit remove redundant common pattern in our XML parsing.
I appreciate splitting the changes for review purposes, most of the time it's a very good practice which makes the reviewer's life easier, but this was a very simple helper with only a handful of replacements, so IMO we can squash this patch into the previous one (there wasn't much to be put into the commit message anyway, so I reckon that's a good indicator :)) I'll squash it into the previous one before pushing: Reviewed-by: Erik Skultety <eskultet@redhat.com>
On 3/13/19 2:30 AM, Shotaro Gotanda wrote:
Changes since v2: - adjust the code to the coding guideline
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 | 21 +++++++++++++++++++++ src/util/virstring.h | 3 +++ 4 files changed, 31 insertions(+), 35 deletions(-)
FYI I updated the bitesizedtask wiki page to describe more cases we can use it. Most of the remaining ones are cases where we only check for "yes", so everything else is implicitly false. In that case if YesNo errors we should just set the value to false and move on https://wiki.libvirt.org/page/BiteSizedTasks#More_usage_of_virStringParseYes... - Cole
participants (3)
-
Cole Robinson -
Erik Skultety -
Shotaro Gotanda