[libvirt PATCH v4 0/8] Refactor XML parsing boilerplate code
This series lays the groundwork for replacing some recurring boilerplate code in src/conf/ regarding the extraction of XML attribute values. For an on / off attribute, the boilerplate code looks roughly like this, g_autofree char *str = NULL; if (str = virXMLPropString(node, "x")) { int val; if ((val = virTristateBoolTypeFromString(str)) <= 0) { virReportError(...) return -1; } def->x = val; } with some variations regarding how `str` is free'd in case of later re-use, the exact error message for invalid values, whether or not `VIR_TRISTATE_(SWITCH|BOOL)_ABSENT` is explicitly checked for (`val < 0` vs. `val <= 0`), whether an intermediate variable is used or the value is assigned directly, and in some cases the conditions in the two if-blocks are merged. After the refactoring, the above code block looks like this: if (virXMLPropTristateBool(node, "x", VIR_XML_PROP_OPTIONAL, &def->x) < 0) return -1; Similar functions are introduced for integer valued attributes, unsigned integer valued attributes and enum valued attributes. Patches #6, #7, and #8 demonstrate the application of these function and stand representative of more patches that I did not sent along yet as to not drown the mailing list in spam. These patches remove a total of ~ 1000 lines of code and fix some places, where e.g. `<foo bar="default"/>` would incorrectly be accepted as virXMLTristateBool. As an added benefit, this refactoring makes the error messages for invalid values in these XML attributes much more consistent: $ git diff master | grep "^-.*_(\"" | wc -l 239 $ git diff master | grep "^+.*_(\"" | wc -l 19 V1: https://listman.redhat.com/archives/libvir-list/2021-March/msg00853.html V2: https://listman.redhat.com/archives/libvir-list/2021-March/msg00994.html V3: https://listman.redhat.com/archives/libvir-list/2021-March/msg01066.html Changes since V3: * Changed virXMLProp* to use bitwise OR-ed enums instead of several boolean arguments * Added virXMLProp(Int|UInt|Enum) * Hold back on most of the actual refactoring to get feedback on the new functions' signatures first Cheers, Tim Tim Wiederhake (8): virxml: Add virXMLPropTristateBool virxml: Add virXMLPropTristateSwitch virxml: Add virXMLPropInt virxml: Add virXMLPropUInt virxml: Add virXMLPropEnum virNetworkForwardNatDefParseXML: Use virXMLProp* virDomainIOThreadIDDefParseXML: Use virXMLProp* virCPUDefParseXML: Use virXMLProp* src/conf/cpu_conf.c | 14 +-- src/conf/domain_conf.c | 14 +-- src/conf/network_conf.c | 16 +-- src/libvirt_private.syms | 5 + src/util/virxml.c | 255 +++++++++++++++++++++++++++++++++++++++ src/util/virxml.h | 31 +++++ 6 files changed, 300 insertions(+), 35 deletions(-) -- 2.26.2
Convenience function to return the value of a yes / no XML attribute. Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virxml.c | 49 ++++++++++++++++++++++++++++++++++++++++ src/util/virxml.h | 10 ++++++++ 3 files changed, 60 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index cb9fe7c80a..1aac0de1ae 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3546,6 +3546,7 @@ virXMLParseHelper; virXMLPickShellSafeComment; virXMLPropString; virXMLPropStringLimit; +virXMLPropTristateBool; virXMLSaveFile; virXMLValidateAgainstSchema; virXMLValidatorFree; diff --git a/src/util/virxml.c b/src/util/virxml.c index 4a6fe09468..0b822d7c4d 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -558,6 +558,55 @@ virXMLNodeContentString(xmlNodePtr node) } +/** + * virXMLPropTristateBool: + * @node: XML dom node pointer + * @name: Name of the property (attribute) to get + * @flags: Bitwise or of virXMLPropFlags + * @result: The returned value + * + * Convenience function to return value of a yes / no attribute. + * + * Returns 1 in case of success in which case @result is set, + * or 0 if the attribute is not present, + * or -1 and reports an error on failure. + */ +int +virXMLPropTristateBool(xmlNodePtr node, const char* name, + virXMLPropFlags flags, virTristateBool *result) +{ + g_autofree char *tmp = NULL; + int val; + + if (!node || !name || !result) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid argument to %s"), + __FUNCTION__); + return -1; + } + + if (!(tmp = virXMLPropString(node, name))) { + if ((flags & VIR_XML_PROP_REQUIRED) != VIR_XML_PROP_REQUIRED) + return 0; + + virReportError(VIR_ERR_XML_ERROR, + _("Missing required attribute '%s' in element '%s'"), + name, node->name); + return -1; + } + + if ((val = virTristateBoolTypeFromString(tmp)) <= 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid value for attribute '%s' in element '%s': '%s'. Expected 'yes' or 'no'"), + name, node->name, tmp); + return -1; + } + + *result = val; + return 1; +} + + /** * virXPathBoolean: * @xpath: the XPath string to evaluate diff --git a/src/util/virxml.h b/src/util/virxml.h index d32f77b867..53de416a7f 100644 --- a/src/util/virxml.h +++ b/src/util/virxml.h @@ -28,10 +28,16 @@ #include <libxml/relaxng.h> #include "virbuffer.h" +#include "virenum.h" xmlXPathContextPtr virXMLXPathContextNew(xmlDocPtr xml) G_GNUC_WARN_UNUSED_RESULT; +typedef enum { + VIR_XML_PROP_OPTIONAL = 0, /* Attribute may be absent */ + VIR_XML_PROP_REQUIRED = 1 << 0, /* Attribute may not be absent */ +} virXMLPropFlags; + int virXPathBoolean(const char *xpath, xmlXPathContextPtr ctxt); char * virXPathString(const char *xpath, @@ -77,6 +83,10 @@ char * virXMLPropStringLimit(xmlNodePtr node, const char *name, size_t maxlen); char * virXMLNodeContentString(xmlNodePtr node); +int virXMLPropTristateBool(xmlNodePtr node, + const char *name, + virXMLPropFlags flags, + virTristateBool *result); /* Internal function; prefer the macros below. */ xmlDocPtr virXMLParseHelper(int domcode, -- 2.26.2
On Thu, Apr 08, 2021 at 10:57:03 +0200, Tim Wiederhake wrote:
Convenience function to return the value of a yes / no XML attribute.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virxml.c | 49 ++++++++++++++++++++++++++++++++++++++++ src/util/virxml.h | 10 ++++++++ 3 files changed, 60 insertions(+)
[...]
diff --git a/src/util/virxml.c b/src/util/virxml.c index 4a6fe09468..0b822d7c4d 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -558,6 +558,55 @@ virXMLNodeContentString(xmlNodePtr node) }
+/** + * virXMLPropTristateBool: + * @node: XML dom node pointer + * @name: Name of the property (attribute) to get + * @flags: Bitwise or of virXMLPropFlags + * @result: The returned value + * + * Convenience function to return value of a yes / no attribute. + * + * Returns 1 in case of success in which case @result is set, + * or 0 if the attribute is not present, + * or -1 and reports an error on failure. + */ +int +virXMLPropTristateBool(xmlNodePtr node, const char* name, + virXMLPropFlags flags, virTristateBool *result)
One argument per line please.
+{ + g_autofree char *tmp = NULL; + int val; + + if (!node || !name || !result) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid argument to %s"), + __FUNCTION__);
Preferably declare the function with ATTRIBUTE_NONNULL instead of this check. Additionally virReportError already records the function name at least in the log message.
+ return -1; + } + + if (!(tmp = virXMLPropString(node, name))) { + if ((flags & VIR_XML_PROP_REQUIRED) != VIR_XML_PROP_REQUIRED)
The part after the masking (!= ... ) is redundant since you mask only one bit out.
+ return 0; + + virReportError(VIR_ERR_XML_ERROR, + _("Missing required attribute '%s' in element '%s'"), + name, node->name); + return -1; + } + + if ((val = virTristateBoolTypeFromString(tmp)) <= 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid value for attribute '%s' in element '%s': '%s'. Expected 'yes' or 'no'"), + name, node->name, tmp); + return -1; + } + + *result = val; + return 1; +} + + /** * virXPathBoolean: * @xpath: the XPath string to evaluate diff --git a/src/util/virxml.h b/src/util/virxml.h index d32f77b867..53de416a7f 100644 --- a/src/util/virxml.h +++ b/src/util/virxml.h @@ -28,10 +28,16 @@ #include <libxml/relaxng.h>
#include "virbuffer.h" +#include "virenum.h"
xmlXPathContextPtr virXMLXPathContextNew(xmlDocPtr xml) G_GNUC_WARN_UNUSED_RESULT;
+typedef enum { + VIR_XML_PROP_OPTIONAL = 0, /* Attribute may be absent */ + VIR_XML_PROP_REQUIRED = 1 << 0, /* Attribute may not be absent */ +} virXMLPropFlags;
Please avoid whitespace alignments.
+ int virXPathBoolean(const char *xpath, xmlXPathContextPtr ctxt); char * virXPathString(const char *xpath, @@ -77,6 +83,10 @@ char * virXMLPropStringLimit(xmlNodePtr node, const char *name, size_t maxlen); char * virXMLNodeContentString(xmlNodePtr node); +int virXMLPropTristateBool(xmlNodePtr node, + const char *name, + virXMLPropFlags flags, + virTristateBool *result);
Same here, please declare the function using the new formatting style, or at least avoid the extraneous pointless whitespace.
/* Internal function; prefer the macros below. */ xmlDocPtr virXMLParseHelper(int domcode, -- 2.26.2
Convenience function to return the value of an on / off XML attribute. Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virxml.c | 49 ++++++++++++++++++++++++++++++++++++++++ src/util/virxml.h | 4 ++++ 3 files changed, 54 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 1aac0de1ae..776387e6b3 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3547,6 +3547,7 @@ virXMLPickShellSafeComment; virXMLPropString; virXMLPropStringLimit; virXMLPropTristateBool; +virXMLPropTristateSwitch; virXMLSaveFile; virXMLValidateAgainstSchema; virXMLValidatorFree; diff --git a/src/util/virxml.c b/src/util/virxml.c index 0b822d7c4d..f62c5c39c4 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -607,6 +607,55 @@ virXMLPropTristateBool(xmlNodePtr node, const char* name, } +/** + * virXMLPropTristateSwitch: + * @node: XML dom node pointer + * @name: Name of the property (attribute) to get + * @flags: Bitwise or of virXMLPropFlags + * @result: The returned value + * + * Convenience function to return value of an on / off attribute. + * + * Returns 1 in case of success in which case @result is set, + * or 0 if the attribute is not present, + * or -1 and reports an error on failure. + */ +int +virXMLPropTristateSwitch(xmlNodePtr node, const char* name, + virXMLPropFlags flags, virTristateSwitch *result) +{ + g_autofree char *tmp = NULL; + int val; + + if (!node || !name || !result) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid argument to %s"), + __FUNCTION__); + return -1; + } + + if (!(tmp = virXMLPropString(node, name))) { + if ((flags & VIR_XML_PROP_REQUIRED) != VIR_XML_PROP_REQUIRED) + return 0; + + virReportError(VIR_ERR_XML_ERROR, + _("Missing required attribute '%s' in element '%s'"), + name, node->name); + return -1; + } + + if ((val = virTristateSwitchTypeFromString(tmp)) <= 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid value for attribute '%s' in element '%s': '%s'. Expected 'on' or 'off'"), + name, node->name, tmp); + return -1; + } + + *result = val; + return 1; +} + + /** * virXPathBoolean: * @xpath: the XPath string to evaluate diff --git a/src/util/virxml.h b/src/util/virxml.h index 53de416a7f..a5ecfbb01a 100644 --- a/src/util/virxml.h +++ b/src/util/virxml.h @@ -87,6 +87,10 @@ int virXMLPropTristateBool(xmlNodePtr node, const char *name, virXMLPropFlags flags, virTristateBool *result); +int virXMLPropTristateSwitch(xmlNodePtr node, + const char *name, + virXMLPropFlags flags, + virTristateSwitch *result); /* Internal function; prefer the macros below. */ xmlDocPtr virXMLParseHelper(int domcode, -- 2.26.2
On Thu, Apr 08, 2021 at 10:57:04 +0200, Tim Wiederhake wrote:
Convenience function to return the value of an on / off XML attribute.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virxml.c | 49 ++++++++++++++++++++++++++++++++++++++++ src/util/virxml.h | 4 ++++ 3 files changed, 54 insertions(+)
See comments on previous patch. Otherwise looks good so you can use my Reviewed-by: Peter Krempa <pkrempa@redhat.com> if you fix it similarly to the previously requested changes.
Convenience function to return the value of an integer XML attribute. Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virxml.c | 51 ++++++++++++++++++++++++++++++++++++++++ src/util/virxml.h | 6 +++++ 3 files changed, 58 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 776387e6b3..bdb63349ee 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3544,6 +3544,7 @@ virXMLNodeSanitizeNamespaces; virXMLNodeToString; virXMLParseHelper; virXMLPickShellSafeComment; +virXMLPropInt; virXMLPropString; virXMLPropStringLimit; virXMLPropTristateBool; diff --git a/src/util/virxml.c b/src/util/virxml.c index f62c5c39c4..807cb7dae9 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -656,6 +656,57 @@ virXMLPropTristateSwitch(xmlNodePtr node, const char* name, } +/** + * virXMLPropInt: + * @node: XML dom node pointer + * @name: Name of the property (attribute) to get + * @base: Number base, see strtol + * @flags: Bitwise or of virXMLPropFlags + * @result: The returned value + * + * Convenience function to return value of an integer attribute. + * + * Returns 1 in case of success in which case @result is set, + * or 0 if the attribute is not present, + * or -1 and reports an error on failure. + */ +int +virXMLPropInt(xmlNodePtr node, const char *name, int base, + virXMLPropFlags flags, int *result) +{ + g_autofree char *tmp = NULL; + int ret; + + if (!node || !name || !result) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid argument to %s"), + __FUNCTION__); + return -1; + } + + if (!(tmp = virXMLPropString(node, name))) { + if ((flags & VIR_XML_PROP_REQUIRED) != VIR_XML_PROP_REQUIRED) + return 0; + + virReportError(VIR_ERR_XML_ERROR, + _("Missing required attribute '%s' in element '%s'"), + name, node->name); + return -1; + } + + ret = virStrToLong_i(tmp, NULL, base, result); + + if ((ret < 0) || ((flags & VIR_XML_PROP_NONZERO) && (*result == 0))) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid value for attribute '%s' in element '%s': '%s'. Expected integer value"), + name, node->name, tmp); + return -1; + } + + return 1; +} + + /** * virXPathBoolean: * @xpath: the XPath string to evaluate diff --git a/src/util/virxml.h b/src/util/virxml.h index a5ecfbb01a..aba863ae11 100644 --- a/src/util/virxml.h +++ b/src/util/virxml.h @@ -36,6 +36,7 @@ xmlXPathContextPtr virXMLXPathContextNew(xmlDocPtr xml) typedef enum { VIR_XML_PROP_OPTIONAL = 0, /* Attribute may be absent */ VIR_XML_PROP_REQUIRED = 1 << 0, /* Attribute may not be absent */ + VIR_XML_PROP_NONZERO = 1 << 1, /* Attribute may not be zero */ } virXMLPropFlags; int virXPathBoolean(const char *xpath, @@ -91,6 +92,11 @@ int virXMLPropTristateSwitch(xmlNodePtr node, const char *name, virXMLPropFlags flags, virTristateSwitch *result); +int virXMLPropInt(xmlNodePtr node, + const char *name, + int base, + virXMLPropFlags flags, + int *result); /* Internal function; prefer the macros below. */ xmlDocPtr virXMLParseHelper(int domcode, -- 2.26.2
On Thu, Apr 08, 2021 at 10:57:05AM +0200, Tim Wiederhake wrote:
Convenience function to return the value of an integer XML attribute.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virxml.c | 51 ++++++++++++++++++++++++++++++++++++++++ src/util/virxml.h | 6 +++++ 3 files changed, 58 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 776387e6b3..bdb63349ee 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3544,6 +3544,7 @@ virXMLNodeSanitizeNamespaces; virXMLNodeToString; virXMLParseHelper; virXMLPickShellSafeComment; +virXMLPropInt; virXMLPropString; virXMLPropStringLimit; virXMLPropTristateBool; diff --git a/src/util/virxml.c b/src/util/virxml.c index f62c5c39c4..807cb7dae9 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -656,6 +656,57 @@ virXMLPropTristateSwitch(xmlNodePtr node, const char* name, }
+/** + * virXMLPropInt: + * @node: XML dom node pointer + * @name: Name of the property (attribute) to get + * @base: Number base, see strtol + * @flags: Bitwise or of virXMLPropFlags + * @result: The returned value + * + * Convenience function to return value of an integer attribute. + * + * Returns 1 in case of success in which case @result is set, + * or 0 if the attribute is not present, + * or -1 and reports an error on failure. + */ +int +virXMLPropInt(xmlNodePtr node, const char *name, int base, + virXMLPropFlags flags, int *result) +{ + g_autofree char *tmp = NULL; + int ret; + + if (!node || !name || !result) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid argument to %s"), + __FUNCTION__); + return -1; + } + + if (!(tmp = virXMLPropString(node, name))) { + if ((flags & VIR_XML_PROP_REQUIRED) != VIR_XML_PROP_REQUIRED) + return 0; + + virReportError(VIR_ERR_XML_ERROR, + _("Missing required attribute '%s' in element '%s'"), + name, node->name); + return -1; + } + + ret = virStrToLong_i(tmp, NULL, base, result); + + if ((ret < 0) || ((flags & VIR_XML_PROP_NONZERO) && (*result == 0))) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid value for attribute '%s' in element '%s': '%s'. Expected integer value"), + name, node->name, tmp); + return -1; + }
0 is an integer value, so this message is not especially helpful in the case VIR_XML_PROP_NONZERO is set. Separate the two tests and give them separate error messages which are more directly relevant. "Unable to parse '%s' for property '%s' as an integer value" "Zero is not a permitted value for property '%s'" Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Convenience function to return the value of an unsigned integer XML attribute. Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virxml.c | 55 ++++++++++++++++++++++++++++++++++++++++ src/util/virxml.h | 6 +++++ 3 files changed, 62 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index bdb63349ee..dc6c9191c8 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3549,6 +3549,7 @@ virXMLPropString; virXMLPropStringLimit; virXMLPropTristateBool; virXMLPropTristateSwitch; +virXMLPropUInt; virXMLSaveFile; virXMLValidateAgainstSchema; virXMLValidatorFree; diff --git a/src/util/virxml.c b/src/util/virxml.c index 807cb7dae9..4d8cda3985 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -707,6 +707,61 @@ virXMLPropInt(xmlNodePtr node, const char *name, int base, } +/** + * virXMLPropUInt: + * @node: XML dom node pointer + * @name: Name of the property (attribute) to get + * @base: Number base, see strtol + * @flags: Bitwise or of virXMLPropFlags + * @result: The returned value + * + * Convenience function to return value of an unsigned integer attribute. + * + * Returns 1 in case of success in which case @result is set, + * or 0 if the attribute is not present, + * or -1 and reports an error on failure. + */ +int +virXMLPropUInt(xmlNodePtr node, const char* name, int base, + virXMLPropFlags flags, unsigned int *result) +{ + g_autofree char *tmp = NULL; + int ret; + + if (!node || !name || !result) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid argument to %s"), + __FUNCTION__); + return -1; + } + + if (!(tmp = virXMLPropString(node, name))) { + if ((flags & VIR_XML_PROP_REQUIRED) != VIR_XML_PROP_REQUIRED) + return 0; + + virReportError(VIR_ERR_XML_ERROR, + _("Missing required attribute '%s' in element '%s'"), + name, node->name); + return -1; + } + + if (flags & VIR_XML_PROP_WRAPNEGATIVE) { + ret = virStrToLong_ui(tmp, NULL, base, result); + } else { + ret = virStrToLong_uip(tmp, NULL, base, result); + } + + if ((ret < 0) || ((flags & VIR_XML_PROP_NONZERO) && (*result == 0))) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid value for attribute '%s' in element '%s': '%s'. Expected integer value"), + name, node->name, tmp); + return -1; + } + + return 1; +} + + /** * virXPathBoolean: * @xpath: the XPath string to evaluate diff --git a/src/util/virxml.h b/src/util/virxml.h index aba863ae11..82d5d16375 100644 --- a/src/util/virxml.h +++ b/src/util/virxml.h @@ -37,6 +37,7 @@ typedef enum { VIR_XML_PROP_OPTIONAL = 0, /* Attribute may be absent */ VIR_XML_PROP_REQUIRED = 1 << 0, /* Attribute may not be absent */ VIR_XML_PROP_NONZERO = 1 << 1, /* Attribute may not be zero */ + VIR_XML_PROP_WRAPNEGATIVE = 1 << 2, /* Wrap around negative values */ } virXMLPropFlags; int virXPathBoolean(const char *xpath, @@ -97,6 +98,11 @@ int virXMLPropInt(xmlNodePtr node, int base, virXMLPropFlags flags, int *result); +int virXMLPropUInt(xmlNodePtr node, + const char* name, + int base, + virXMLPropFlags flags, + unsigned int *result); /* Internal function; prefer the macros below. */ xmlDocPtr virXMLParseHelper(int domcode, -- 2.26.2
Convenience function to return the value of an enum XML attribute. Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virxml.c | 51 ++++++++++++++++++++++++++++++++++++++++ src/util/virxml.h | 5 ++++ 3 files changed, 57 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index dc6c9191c8..79eb4dba50 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3544,6 +3544,7 @@ virXMLNodeSanitizeNamespaces; virXMLNodeToString; virXMLParseHelper; virXMLPickShellSafeComment; +virXMLPropEnum; virXMLPropInt; virXMLPropString; virXMLPropStringLimit; diff --git a/src/util/virxml.c b/src/util/virxml.c index 4d8cda3985..a994a404d4 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -762,6 +762,57 @@ virXMLPropUInt(xmlNodePtr node, const char* name, int base, } +/** + * virXMLPropEnum: + * @node: XML dom node pointer + * @name: Name of the property (attribute) to get + * @strToInt: Conversion function to turn enum name to value. Expected to + * return negative value on failure. + * @flags: Bitwise or of virXMLPropFlags + * @result: The returned value + * + * Convenience function to return value of an enum attribute. + * + * Returns 1 in case of success in which case @result is set, + * or 0 if the attribute is not present, + * or -1 and reports an error on failure. + */ +int +virXMLPropEnum(xmlNodePtr node, const char* name, int (*strToInt)(const char*), + virXMLPropFlags flags, unsigned int *result) +{ + g_autofree char *tmp = NULL; + int ret; + + if (!node || !name || !strToInt || !result) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid argument to %s"), + __FUNCTION__); + return -1; + } + + if (!(tmp = virXMLPropString(node, name))) { + if ((flags & VIR_XML_PROP_REQUIRED) != VIR_XML_PROP_REQUIRED) + return 0; + + virReportError(VIR_ERR_XML_ERROR, + _("Missing required attribute '%s' in element '%s'"), + name, node->name); + return -1; + } + + ret = strToInt(tmp); + if ((ret < 0) || ((flags & VIR_XML_PROP_NONZERO) && (ret == 0))) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid value for attribute '%s' in element '%s': '%s'."), + name, node->name, NULLSTR(tmp)); + return -1; + } + + *result = ret; + return 0; +} + /** * virXPathBoolean: * @xpath: the XPath string to evaluate diff --git a/src/util/virxml.h b/src/util/virxml.h index 82d5d16375..cbe4cc32dc 100644 --- a/src/util/virxml.h +++ b/src/util/virxml.h @@ -103,6 +103,11 @@ int virXMLPropUInt(xmlNodePtr node, int base, virXMLPropFlags flags, unsigned int *result); +int virXMLPropEnum(xmlNodePtr node, + const char* name, + int (*strToInt)(const char*), + virXMLPropFlags flags, + unsigned int *result); /* Internal function; prefer the macros below. */ xmlDocPtr virXMLParseHelper(int domcode, -- 2.26.2
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/network_conf.c | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 4cf4aa4840..19b84cbdd1 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -1321,7 +1321,6 @@ virNetworkForwardNatDefParseXML(const char *networkName, g_autofree xmlNodePtr *natPortNodes = NULL; g_autofree char *addrStart = NULL; g_autofree char *addrEnd = NULL; - g_autofree char *ipv6 = NULL; VIR_XPATH_NODE_AUTORESTORE(ctxt) ctxt->node = node; @@ -1333,18 +1332,9 @@ virNetworkForwardNatDefParseXML(const char *networkName, return -1; } - ipv6 = virXMLPropString(node, "ipv6"); - if (ipv6) { - int natIPv6; - if ((natIPv6 = virTristateBoolTypeFromString(ipv6)) <= 0) { - virReportError(VIR_ERR_XML_ERROR, - _("Invalid ipv6 setting '%s' " - "in network '%s' NAT"), - ipv6, networkName); - return -1; - } - def->natIPv6 = natIPv6; - } + if (virXMLPropTristateBool(node, "ipv6", VIR_XML_PROP_OPTIONAL, + &def->natIPv6) < 0) + return -1; /* addresses for SNAT */ nNatAddrs = virXPathNodeSet("./address", ctxt, &natAddrNodes); -- 2.26.2
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1e72171586..fb22695dd3 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -18180,21 +18180,13 @@ static virDomainIOThreadIDDefPtr virDomainIOThreadIDDefParseXML(xmlNodePtr node) { virDomainIOThreadIDDefPtr iothrid; - g_autofree char *tmp = NULL; iothrid = g_new0(virDomainIOThreadIDDef, 1); - if (!(tmp = virXMLPropString(node, "id"))) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("Missing 'id' attribute in <iothread> element")); - goto error; - } - if (virStrToLong_uip(tmp, NULL, 10, &iothrid->iothread_id) < 0 || - iothrid->iothread_id == 0) { - virReportError(VIR_ERR_XML_ERROR, - _("invalid iothread 'id' value '%s'"), tmp); + if (virXMLPropUInt(node, "id", 10, + VIR_XML_PROP_REQUIRED | VIR_XML_PROP_NONZERO, + &iothrid->iothread_id) < 0) goto error; - } return iothrid; -- 2.26.2
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/cpu_conf.c | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index 43629068c3..ee7feb1186 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -423,7 +423,6 @@ virCPUDefParseXML(xmlXPathContextPtr ctxt, if (def->type == VIR_CPU_TYPE_GUEST) { g_autofree char *match = virXMLPropString(ctxt->node, "match"); - g_autofree char *check = NULL; if (match) { def->match = virCPUMatchTypeFromString(match); @@ -435,16 +434,9 @@ virCPUDefParseXML(xmlXPathContextPtr ctxt, } } - if ((check = virXMLPropString(ctxt->node, "check"))) { - int value = virCPUCheckTypeFromString(check); - if (value < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Invalid check attribute for CPU " - "specification")); - return -1; - } - def->check = value; - } + if (virXMLPropEnum(ctxt->node, "check", virCPUCheckTypeFromString, + VIR_XML_PROP_OPTIONAL, &def->check) < 0) + return -1; } if (def->type == VIR_CPU_TYPE_HOST) { -- 2.26.2
participants (3)
-
Daniel P. Berrangé -
Peter Krempa -
Tim Wiederhake