[PATCH 00/43] Remove XPath parser functions using 'long' type
Clean up the XML parser functions to avoid use of the long type. Peter Krempa (43): util: xml: Remove unused virXPathNumber util: xml: Unexport virXMLXPathContextNew util: xml: Ensure proper header style in virxml.c util: xml: Use consistent naming for RNG validation error handling functions util: xml: Remove unused 'virXPathLongHex' virNodeDevCapsDefParseULong: Use virXPathUInt instead of virXPathULong conf: node_device: Rename virNodeDevCapsDefParseULong to virNodeDevCapsDefParseUInt virNodeDeviceCapPCIDefFormat: Use %u for unsigned values conf: Always use 'string()' conversion with virXPath(U)LongLong util: xml: Extract XPath evaluation for strings util: xml: Remove double->(u)ll conversion in virXPath(U)LongLong util: xml: Disallow aliasing of negative numbers in virXPathULongLong conf: node_device: Use 'string()' in XPath expressions for virNodeDevCapsDefParseUInt conf: node_device: Convert rest of virXPathUInt XPath expressions to number conf: node_device: Use 'string()' in XPath expressions for virNodeDevCapsDefParseIntOptional conf: numa: Don't fetch XML node count in virDomainNumatuneParseXML conf: cpu: Extract and refactor parsing of cache from virCPUDefParseXML util: xml: Reimplement virXPath(U)Int via virXPathEvalString util: xml: Introduce virXPathU(Int|LongLong)Base virNodeDevCapsDefParseHexId: Use 'virXPathUIntBase' util: xml: Disallow aliasing of negative numbers in virXPathUInt testParseNodeInfo: Rewrite to virXPathU(Int|LongLong) conf: domain: Convert from virXPathLong util: xml: Remove virXPathLong virQEMUCapsLoadCache: Use 'virXMLPropUInt' instead of 'virXPathULong' virNetDevIPRouteParseXML: Refactor to use 'virXMLProp*' instead of XPath virNetworkIPDefParseXML: Use virXMLPropUInt instead of virXPathULong virInterfaceDefParseMtu: Use virXPathUInt instead of virXPathULong virNetDevVlanParse: Use virXMLProp* helpers instead of XPath lookups virDomainTimerCatchupDef: Change members to 'unsigned long long' virDomainTimerDefParseXML: Refactor cleanup virDomainTimerDef: Convert 'name' field to proper enum type virDomainTimerDef: Convert 'tickpolicy' field to proper enum type virDomainTimerDef: Convert 'track' field to proper enum type virDomainTimerDef: Convert 'mode' field to proper enum type virDomainTimerDefParseXML: Use virXMLProp instead of virXPath virDomainNetDef: Change type of 'tune.sndbuf' virDomainSEVDefParseXML: Use virXPathUIntBase instead of virXPathULongHex ppc64ModelParse: Switch to virXMLPropUInt from virXPathULongHex qemuDomainObjPrivateXMLParseBlockjobData: Use virXMLPropUInt instead of virXPathULongHex virDomainJobObj: Use 'unsigned int' instead of 'unsigned long' for 'apiFlags' field cpu_arm: Avoid use of 'unsigned long' util: xml: Remove unused virXPathULong* src/conf/cpu_conf.c | 58 +-- src/conf/domain_conf.c | 153 +++----- src/conf/domain_conf.h | 16 +- src/conf/interface_conf.c | 14 +- src/conf/netdev_vlan_conf.c | 39 +- src/conf/network_conf.c | 25 +- src/conf/networkcommon_conf.c | 61 +-- src/conf/networkcommon_conf.h | 3 +- src/conf/node_device_conf.c | 160 ++++---- src/conf/numa_conf.c | 7 +- src/conf/storage_conf.c | 4 +- src/conf/virdomainjob.c | 4 +- src/conf/virdomainjob.h | 4 +- src/cpu/cpu_arm.c | 37 +- src/cpu/cpu_arm_data.h | 4 +- src/cpu/cpu_ppc64.c | 19 +- src/libvirt_private.syms | 8 +- src/libxl/libxl_conf.c | 6 +- src/libxl/xen_common.c | 6 +- src/qemu/qemu_capabilities.c | 7 +- src/qemu/qemu_command.c | 7 + src/qemu/qemu_domain.c | 5 +- src/qemu/qemu_domainjob.c | 7 +- src/qemu/qemu_migration.c | 2 +- src/qemu/qemu_migration_params.c | 8 +- src/qemu/qemu_migration_params.h | 4 +- src/qemu/qemu_process.c | 2 +- src/qemu/qemu_process.h | 2 +- src/qemu/qemu_validate.c | 8 + src/test/test_driver.c | 51 +-- src/util/virxml.c | 370 ++++++------------ src/util/virxml.h | 32 +- .../pci_0000_00_02_0_header_type.xml | 2 +- 33 files changed, 432 insertions(+), 703 deletions(-) -- 2.37.3
'virXPathNumber' is not used currently, remove it. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt_private.syms | 1 - src/util/virxml.c | 33 --------------------------------- src/util/virxml.h | 4 ---- 3 files changed, 38 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 0b0ccbafe5..61003eff39 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3699,7 +3699,6 @@ virXPathLongHex; virXPathLongLong; virXPathNode; virXPathNodeSet; -virXPathNumber; virXPathString; virXPathUInt; virXPathULong; diff --git a/src/util/virxml.c b/src/util/virxml.c index 0548a29e65..d99b060b63 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -90,39 +90,6 @@ virXPathString(const char *xpath, } -/** - * virXPathNumber: - * @xpath: the XPath string to evaluate - * @ctxt: an XPath context - * @value: the returned double value - * - * Convenience function to evaluate an XPath number - * - * Returns 0 in case of success in which case @value is set, - * or -1 if the evaluation failed. - */ -int -virXPathNumber(const char *xpath, - xmlXPathContextPtr ctxt, - double *value) -{ - g_autoptr(xmlXPathObject) obj = NULL; - - if ((ctxt == NULL) || (xpath == NULL) || (value == NULL)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("Invalid parameter to virXPathNumber()")); - return -1; - } - obj = xmlXPathEval(BAD_CAST xpath, ctxt); - if ((obj == NULL) || (obj->type != XPATH_NUMBER) || - (isnan(obj->floatval))) { - return -1; - } - - *value = obj->floatval; - return 0; -} - static int virXPathLongBase(const char *xpath, xmlXPathContextPtr ctxt, diff --git a/src/util/virxml.h b/src/util/virxml.h index febc7cbdfb..6d61d91497 100644 --- a/src/util/virxml.h +++ b/src/util/virxml.h @@ -51,10 +51,6 @@ char * virXPathString(const char *xpath, xmlXPathContextPtr ctxt); int -virXPathNumber(const char *xpath, - xmlXPathContextPtr ctxt, - double *value); -int virXPathInt(const char *xpath, xmlXPathContextPtr ctxt, int *value); -- 2.37.3
On a Monday in 2022, Peter Krempa wrote:
'virXPathNumber' is not used currently, remove it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt_private.syms | 1 - src/util/virxml.c | 33 --------------------------------- src/util/virxml.h | 4 ---- 3 files changed, 38 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
The function is now referenced only within util/virxml.c other callers should not use it directly. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt_private.syms | 1 - src/util/virxml.c | 2 +- src/util/virxml.h | 4 ---- 3 files changed, 1 insertion(+), 6 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 61003eff39..7150e87c38 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3690,7 +3690,6 @@ virXMLValidateAgainstSchema; virXMLValidatorFree; virXMLValidatorInit; virXMLValidatorValidate; -virXMLXPathContextNew; virXPathBoolean; virXPathContextNodeRestore; virXPathInt; diff --git a/src/util/virxml.c b/src/util/virxml.c index d99b060b63..de7b7a095d 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -48,7 +48,7 @@ struct virParserData { }; -xmlXPathContextPtr +static xmlXPathContextPtr virXMLXPathContextNew(xmlDocPtr xml) { xmlXPathContextPtr ctxt; diff --git a/src/util/virxml.h b/src/util/virxml.h index 6d61d91497..8a03077695 100644 --- a/src/util/virxml.h +++ b/src/util/virxml.h @@ -30,10 +30,6 @@ #include "virbuffer.h" #include "virenum.h" -xmlXPathContextPtr virXMLXPathContextNew(xmlDocPtr xml) - G_GNUC_WARN_UNUSED_RESULT; - - typedef enum { VIR_XML_PROP_NONE = 0, VIR_XML_PROP_REQUIRED = 1 << 0, /* Attribute may not be absent */ -- 2.37.3
On a Monday in 2022, Peter Krempa wrote:
The function is now referenced only within util/virxml.c other callers should not use it directly.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt_private.syms | 1 - src/util/virxml.c | 2 +- src/util/virxml.h | 4 ---- 3 files changed, 1 insertion(+), 6 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Make the file use consistent header formatting and two line spacing between functions. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virxml.c | 44 ++++++++++++++++++++++++++++++++++---------- 1 file changed, 34 insertions(+), 10 deletions(-) diff --git a/src/util/virxml.c b/src/util/virxml.c index de7b7a095d..43b1ccfd24 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -121,6 +121,7 @@ virXPathLongBase(const char *xpath, return ret; } + /** * virXPathInt: * @xpath: the XPath string to evaluate @@ -150,6 +151,7 @@ virXPathInt(const char *xpath, return 0; } + /** * virXPathLong: * @xpath: the XPath string to evaluate @@ -170,6 +172,7 @@ virXPathLong(const char *xpath, return virXPathLongBase(xpath, ctxt, 10, value); } + /** * virXPathLongHex: * @xpath: the XPath string to evaluate @@ -191,6 +194,7 @@ virXPathLongHex(const char *xpath, return virXPathLongBase(xpath, ctxt, 16, value); } + static int virXPathULongBase(const char *xpath, xmlXPathContextPtr ctxt, @@ -222,6 +226,7 @@ virXPathULongBase(const char *xpath, return ret; } + /** * virXPathUInt: * @xpath: the XPath string to evaluate @@ -251,6 +256,7 @@ virXPathUInt(const char *xpath, return 0; } + /** * virXPathULong: * @xpath: the XPath string to evaluate @@ -271,6 +277,7 @@ virXPathULong(const char *xpath, return virXPathULongBase(xpath, ctxt, 10, value); } + /** * virXPathUHex: * @xpath: the XPath string to evaluate @@ -292,6 +299,7 @@ virXPathULongHex(const char *xpath, return virXPathULongBase(xpath, ctxt, 16, value); } + /** * virXPathULongLong: * @xpath: the XPath string to evaluate @@ -334,6 +342,7 @@ virXPathULongLong(const char *xpath, return ret; } + /** * virXPathLongLong: * @xpath: the XPath string to evaluate @@ -482,6 +491,7 @@ virXMLNodeContentString(xmlNodePtr node) return ret; } + static int virXMLPropEnumInternal(xmlNodePtr node, const char *name, @@ -967,6 +977,7 @@ virXPathBoolean(const char *xpath, return obj->boolval; } + /** * virXPathNode: * @xpath: the XPath string to evaluate @@ -998,6 +1009,7 @@ virXPathNode(const char *xpath, return obj->nodesetval->nodeTab[0]; } + /** * virXPathNodeSet: * @xpath: the XPath string to evaluate @@ -1133,6 +1145,7 @@ catchXMLError(void *ctx, const char *msg G_GNUC_UNUSED, ...) } } + /** * virXMLParseHelper: * @domcode: error domain of the caller, usually VIR_FROM_THIS @@ -1238,7 +1251,10 @@ virXMLParseHelper(int domcode, return g_steal_pointer(&xml); } -const char *virXMLPickShellSafeComment(const char *str1, const char *str2) + +const char * +virXMLPickShellSafeComment(const char *str1, + const char *str2) { if (str1 && !strpbrk(str1, "\r\t\n !\"#$&'()*;<>?[\\]^`{|}~") && !strstr(str1, "--")) @@ -1249,9 +1265,11 @@ const char *virXMLPickShellSafeComment(const char *str1, const char *str2) return NULL; } -static int virXMLEmitWarning(int fd, - const char *name, - const char *cmd) + +static int +virXMLEmitWarning(int fd, + const char *name, + const char *cmd) { size_t len; const char *prologue = @@ -1300,6 +1318,7 @@ struct virXMLRewriteFileData { const char *xml; }; + static int virXMLRewriteFile(int fd, const char *path, @@ -1325,6 +1344,7 @@ virXMLRewriteFile(int fd, return 0; } + int virXMLSaveFile(const char *path, const char *warnName, @@ -1337,6 +1357,7 @@ virXMLSaveFile(const char *path, virXMLRewriteFile, &data); } + /** * virXMLNodeToString: convert an XML node ptr to an XML string * @@ -1586,9 +1607,10 @@ virXMLNodeSanitizeNamespaces(xmlNodePtr node) } -static void catchRNGError(void *ctx, - const char *msg, - ...) +static void +catchRNGError(void *ctx, + const char *msg, + ...) { virBuffer *buf = ctx; va_list args; @@ -1601,9 +1623,10 @@ static void catchRNGError(void *ctx, } -static void ignoreRNGError(void *ctx G_GNUC_UNUSED, - const char *msg G_GNUC_UNUSED, - ...) +static void +ignoreRNGError(void *ctx G_GNUC_UNUSED, + const char *msg G_GNUC_UNUSED, + ...) {} @@ -1772,6 +1795,7 @@ virXMLFormatElementInternal(virBuffer *buf, virBufferFreeAndReset(childBuf); } + /* same as virXMLFormatElement but outputs an empty element if @attrBuf and * @childBuf are both empty */ void -- 2.37.3
On a Monday in 2022, Peter Krempa wrote:
Make the file use consistent header formatting and two line spacing between functions.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virxml.c | 44 ++++++++++++++++++++++++++++++++++---------- 1 file changed, 34 insertions(+), 10 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Rename 'catchRNGError' to 'virXMLValidatorRNGErrorCatch' and 'ignoreRNGError' to 'virXMLValidatorRNGErrorIgnore'. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virxml.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/util/virxml.c b/src/util/virxml.c index 43b1ccfd24..16e7ef9808 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -1608,9 +1608,9 @@ virXMLNodeSanitizeNamespaces(xmlNodePtr node) static void -catchRNGError(void *ctx, - const char *msg, - ...) +virXMLValidatorRNGErrorCatch(void *ctx, + const char *msg, + ...) { virBuffer *buf = ctx; va_list args; @@ -1624,9 +1624,9 @@ catchRNGError(void *ctx, static void -ignoreRNGError(void *ctx G_GNUC_UNUSED, - const char *msg G_GNUC_UNUSED, - ...) +virXMLValidatorRNGErrorIgnore(void *ctx G_GNUC_UNUSED, + const char *msg G_GNUC_UNUSED, + ...) {} @@ -1648,8 +1648,8 @@ virXMLValidatorInit(const char *schemafile) } xmlRelaxNGSetParserErrors(validator->rngParser, - catchRNGError, - ignoreRNGError, + virXMLValidatorRNGErrorCatch, + virXMLValidatorRNGErrorIgnore, &validator->buf); if (!(validator->rng = xmlRelaxNGParse(validator->rngParser))) { @@ -1668,8 +1668,8 @@ virXMLValidatorInit(const char *schemafile) } xmlRelaxNGSetValidErrors(validator->rngValid, - catchRNGError, - ignoreRNGError, + virXMLValidatorRNGErrorCatch, + virXMLValidatorRNGErrorIgnore, &validator->buf); return validator; -- 2.37.3
On a Monday in 2022, Peter Krempa wrote:
Rename 'catchRNGError' to 'virXMLValidatorRNGErrorCatch' and 'ignoreRNGError' to 'virXMLValidatorRNGErrorIgnore'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virxml.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt_private.syms | 1 - src/util/virxml.c | 22 ---------------------- src/util/virxml.h | 4 ---- 3 files changed, 27 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 7150e87c38..c6445dedcb 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3694,7 +3694,6 @@ virXPathBoolean; virXPathContextNodeRestore; virXPathInt; virXPathLong; -virXPathLongHex; virXPathLongLong; virXPathNode; virXPathNodeSet; diff --git a/src/util/virxml.c b/src/util/virxml.c index 16e7ef9808..e771471dd7 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -173,28 +173,6 @@ virXPathLong(const char *xpath, } -/** - * virXPathLongHex: - * @xpath: the XPath string to evaluate - * @ctxt: an XPath context - * @value: the returned long value - * - * Convenience function to evaluate an XPath number - * according to a base of 16 - * - * Returns 0 in case of success in which case @value is set, - * or -1 if the XPath evaluation failed or -2 if the - * value doesn't have a long format. - */ -int -virXPathLongHex(const char *xpath, - xmlXPathContextPtr ctxt, - long *value) -{ - return virXPathLongBase(xpath, ctxt, 16, value); -} - - static int virXPathULongBase(const char *xpath, xmlXPathContextPtr ctxt, diff --git a/src/util/virxml.h b/src/util/virxml.h index 8a03077695..fe07e2d223 100644 --- a/src/util/virxml.h +++ b/src/util/virxml.h @@ -71,10 +71,6 @@ virXPathLongLong(const char *xpath, xmlXPathContextPtr ctxt, long long *value); int -virXPathLongHex(const char *xpath, - xmlXPathContextPtr ctxt, - long *value); -int virXPathULongHex(const char *xpath, xmlXPathContextPtr ctxt, unsigned long *value); -- 2.37.3
On a Monday in 2022, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt_private.syms | 1 - src/util/virxml.c | 22 ---------------------- src/util/virxml.h | 4 ---- 3 files changed, 27 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Fix the function argument to properly spell out 'unsigned int' and use virXPathUInt instead of virXPathULong and a temporary value. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/node_device_conf.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index bdfbbab434..bb10b9dabe 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -833,15 +833,14 @@ virNodeDevCapsDefParseIntOptional(const char *xpath, static int virNodeDevCapsDefParseULong(const char *xpath, xmlXPathContextPtr ctxt, - unsigned *value, + unsigned int *value, virNodeDeviceDef *def, const char *missing_error_fmt, const char *invalid_error_fmt) { int ret; - unsigned long val; - ret = virXPathULong(xpath, ctxt, &val); + ret = virXPathUInt(xpath, ctxt, value); if (ret < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, ret == -1 ? missing_error_fmt : invalid_error_fmt, @@ -849,7 +848,6 @@ virNodeDevCapsDefParseULong(const char *xpath, return -1; } - *value = val; return 0; } -- 2.37.3
On a Monday in 2022, Peter Krempa wrote:
Fix the function argument to properly spell out 'unsigned int' and use virXPathUInt instead of virXPathULong and a temporary value.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/node_device_conf.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
The function parses an unsigned int so rename it. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/node_device_conf.c | 132 ++++++++++++++++++------------------ 1 file changed, 66 insertions(+), 66 deletions(-) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index bb10b9dabe..9c0a27b9f0 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -831,12 +831,12 @@ virNodeDevCapsDefParseIntOptional(const char *xpath, static int -virNodeDevCapsDefParseULong(const char *xpath, - xmlXPathContextPtr ctxt, - unsigned int *value, - virNodeDeviceDef *def, - const char *missing_error_fmt, - const char *invalid_error_fmt) +virNodeDevCapsDefParseUInt(const char *xpath, + xmlXPathContextPtr ctxt, + unsigned int *value, + virNodeDeviceDef *def, + const char *missing_error_fmt, + const char *invalid_error_fmt) { int ret; @@ -1502,28 +1502,28 @@ virNodeDevCapSCSIParseXML(xmlXPathContextPtr ctxt, ctxt->node = node; - if (virNodeDevCapsDefParseULong("number(./host[1])", ctxt, - &scsi->host, def, - _("no SCSI host ID supplied for '%s'"), - _("invalid SCSI host ID supplied for '%s'")) < 0) + if (virNodeDevCapsDefParseUInt("number(./host[1])", ctxt, + &scsi->host, def, + _("no SCSI host ID supplied for '%s'"), + _("invalid SCSI host ID supplied for '%s'")) < 0) return -1; - if (virNodeDevCapsDefParseULong("number(./bus[1])", ctxt, - &scsi->bus, def, - _("no SCSI bus ID supplied for '%s'"), - _("invalid SCSI bus ID supplied for '%s'")) < 0) + if (virNodeDevCapsDefParseUInt("number(./bus[1])", ctxt, + &scsi->bus, def, + _("no SCSI bus ID supplied for '%s'"), + _("invalid SCSI bus ID supplied for '%s'")) < 0) return -1; - if (virNodeDevCapsDefParseULong("number(./target[1])", ctxt, - &scsi->target, def, - _("no SCSI target ID supplied for '%s'"), - _("invalid SCSI target ID supplied for '%s'")) < 0) + if (virNodeDevCapsDefParseUInt("number(./target[1])", ctxt, + &scsi->target, def, + _("no SCSI target ID supplied for '%s'"), + _("invalid SCSI target ID supplied for '%s'")) < 0) return -1; - if (virNodeDevCapsDefParseULong("number(./lun[1])", ctxt, - &scsi->lun, def, - _("no SCSI LUN ID supplied for '%s'"), - _("invalid SCSI LUN ID supplied for '%s'")) < 0) + if (virNodeDevCapsDefParseUInt("number(./lun[1])", ctxt, + &scsi->lun, def, + _("no SCSI LUN ID supplied for '%s'"), + _("invalid SCSI LUN ID supplied for '%s'")) < 0) return -1; scsi->type = virXPathString("string(./type[1])", ctxt); @@ -1615,10 +1615,10 @@ virNodeDevCapSCSIHostParseXML(xmlXPathContextPtr ctxt, ctxt->node = node; if (create == EXISTING_DEVICE) { - if (virNodeDevCapsDefParseULong("number(./host[1])", ctxt, - &scsi_host->host, def, - _("no SCSI host ID supplied for '%s'"), - _("invalid SCSI host ID supplied for '%s'")) < 0) { + if (virNodeDevCapsDefParseUInt("number(./host[1])", ctxt, + &scsi_host->host, def, + _("no SCSI host ID supplied for '%s'"), + _("invalid SCSI host ID supplied for '%s'")) < 0) { return -1; } /* Optional unique_id value */ @@ -1775,28 +1775,28 @@ virNodeDevCapUSBInterfaceParseXML(xmlXPathContextPtr ctxt, ctxt->node = node; - if (virNodeDevCapsDefParseULong("number(./number[1])", ctxt, - &usb_if->number, def, - _("no USB interface number supplied for '%s'"), - _("invalid USB interface number supplied for '%s'")) < 0) + if (virNodeDevCapsDefParseUInt("number(./number[1])", ctxt, + &usb_if->number, def, + _("no USB interface number supplied for '%s'"), + _("invalid USB interface number supplied for '%s'")) < 0) return -1; - if (virNodeDevCapsDefParseULong("number(./class[1])", ctxt, - &usb_if->klass, def, - _("no USB interface class supplied for '%s'"), - _("invalid USB interface class supplied for '%s'")) < 0) + if (virNodeDevCapsDefParseUInt("number(./class[1])", ctxt, + &usb_if->klass, def, + _("no USB interface class supplied for '%s'"), + _("invalid USB interface class supplied for '%s'")) < 0) return -1; - if (virNodeDevCapsDefParseULong("number(./subclass[1])", ctxt, - &usb_if->subclass, def, - _("no USB interface subclass supplied for '%s'"), - _("invalid USB interface subclass supplied for '%s'")) < 0) + if (virNodeDevCapsDefParseUInt("number(./subclass[1])", ctxt, + &usb_if->subclass, def, + _("no USB interface subclass supplied for '%s'"), + _("invalid USB interface subclass supplied for '%s'")) < 0) return -1; - if (virNodeDevCapsDefParseULong("number(./protocol[1])", ctxt, - &usb_if->protocol, def, - _("no USB interface protocol supplied for '%s'"), - _("invalid USB interface protocol supplied for '%s'")) < 0) + if (virNodeDevCapsDefParseUInt("number(./protocol[1])", ctxt, + &usb_if->protocol, def, + _("no USB interface protocol supplied for '%s'"), + _("invalid USB interface protocol supplied for '%s'")) < 0) return -1; usb_if->description = virXPathString("string(./description[1])", ctxt); @@ -1839,16 +1839,16 @@ virNodeDevCapUSBDevParseXML(xmlXPathContextPtr ctxt, ctxt->node = node; - if (virNodeDevCapsDefParseULong("number(./bus[1])", ctxt, - &usb_dev->bus, def, - _("no USB bus number supplied for '%s'"), - _("invalid USB bus number supplied for '%s'")) < 0) + if (virNodeDevCapsDefParseUInt("number(./bus[1])", ctxt, + &usb_dev->bus, def, + _("no USB bus number supplied for '%s'"), + _("invalid USB bus number supplied for '%s'")) < 0) return -1; - if (virNodeDevCapsDefParseULong("number(./device[1])", ctxt, - &usb_dev->device, def, - _("no USB device number supplied for '%s'"), - _("invalid USB device number supplied for '%s'")) < 0) + if (virNodeDevCapsDefParseUInt("number(./device[1])", ctxt, + &usb_dev->device, def, + _("no USB device number supplied for '%s'"), + _("invalid USB device number supplied for '%s'")) < 0) return -1; if (virNodeDevCapsDefParseHexId("string(./vendor[1]/@id)", ctxt, @@ -2084,28 +2084,28 @@ virNodeDevCapPCIDevParseXML(xmlXPathContextPtr ctxt, pci_dev->klass = -1; } - if (virNodeDevCapsDefParseULong("number(./domain[1])", ctxt, - &pci_dev->domain, def, - _("no PCI domain ID supplied for '%s'"), - _("invalid PCI domain ID supplied for '%s'")) < 0) + if (virNodeDevCapsDefParseUInt("number(./domain[1])", ctxt, + &pci_dev->domain, def, + _("no PCI domain ID supplied for '%s'"), + _("invalid PCI domain ID supplied for '%s'")) < 0) goto out; - if (virNodeDevCapsDefParseULong("number(./bus[1])", ctxt, - &pci_dev->bus, def, - _("no PCI bus ID supplied for '%s'"), - _("invalid PCI bus ID supplied for '%s'")) < 0) + if (virNodeDevCapsDefParseUInt("number(./bus[1])", ctxt, + &pci_dev->bus, def, + _("no PCI bus ID supplied for '%s'"), + _("invalid PCI bus ID supplied for '%s'")) < 0) goto out; - if (virNodeDevCapsDefParseULong("number(./slot[1])", ctxt, - &pci_dev->slot, def, - _("no PCI slot ID supplied for '%s'"), - _("invalid PCI slot ID supplied for '%s'")) < 0) + if (virNodeDevCapsDefParseUInt("number(./slot[1])", ctxt, + &pci_dev->slot, def, + _("no PCI slot ID supplied for '%s'"), + _("invalid PCI slot ID supplied for '%s'")) < 0) goto out; - if (virNodeDevCapsDefParseULong("number(./function[1])", ctxt, - &pci_dev->function, def, - _("no PCI function ID supplied for '%s'"), - _("invalid PCI function ID supplied for '%s'")) < 0) + if (virNodeDevCapsDefParseUInt("number(./function[1])", ctxt, + &pci_dev->function, def, + _("no PCI function ID supplied for '%s'"), + _("invalid PCI function ID supplied for '%s'")) < 0) goto out; if (virNodeDevCapsDefParseHexId("string(./vendor[1]/@id)", ctxt, -- 2.37.3
On a Monday in 2022, Peter Krempa wrote:
The function parses an unsigned int so rename it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/node_device_conf.c | 132 ++++++++++++++++++------------------ 1 file changed, 66 insertions(+), 66 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
'bus', 'slot' and 'function' are unsigned int variables parsed as unsigned int, but were formated as signed. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/node_device_conf.c | 6 +++--- tests/nodedevschemadata/pci_0000_00_02_0_header_type.xml | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 9c0a27b9f0..b079c3713d 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -323,10 +323,10 @@ virNodeDeviceCapPCIDefFormat(virBuffer *buf, virBufferAsprintf(buf, "<class>0x%.6x</class>\n", data->pci_dev.klass); virBufferAsprintf(buf, "<domain>%d</domain>\n", data->pci_dev.domain); - virBufferAsprintf(buf, "<bus>%d</bus>\n", data->pci_dev.bus); - virBufferAsprintf(buf, "<slot>%d</slot>\n", + virBufferAsprintf(buf, "<bus>%u</bus>\n", data->pci_dev.bus); + virBufferAsprintf(buf, "<slot>%u</slot>\n", data->pci_dev.slot); - virBufferAsprintf(buf, "<function>%d</function>\n", + virBufferAsprintf(buf, "<function>%u</function>\n", data->pci_dev.function); virBufferAsprintf(buf, "<product id='0x%04x'", data->pci_dev.product); diff --git a/tests/nodedevschemadata/pci_0000_00_02_0_header_type.xml b/tests/nodedevschemadata/pci_0000_00_02_0_header_type.xml index 387fce7051..df620dc64f 100644 --- a/tests/nodedevschemadata/pci_0000_00_02_0_header_type.xml +++ b/tests/nodedevschemadata/pci_0000_00_02_0_header_type.xml @@ -4,7 +4,7 @@ <capability type='pci'> <class>0xffffff</class> <domain>0</domain> - <bus>0</bus> + <bus>4294967295</bus> <slot>2</slot> <function>0</function> <product id='0x0416'>4th Gen Core Processor Integrated Graphics Controller</product> -- 2.37.3
On a Monday in 2022, Peter Krempa wrote:
'bus', 'slot' and 'function' are unsigned int variables parsed as unsigned int, but were formated as signed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/node_device_conf.c | 6 +++--- tests/nodedevschemadata/pci_0000_00_02_0_header_type.xml | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
When the 'string()' conversion is used the number is parsed inside libvirt by our internal helpers which work on integers in contrast to when 'number()' is used and libxml2 uses a 'double' variable internally. On the upper extremes of the 64 bit variables the doulbe precision variable doesn't have enough precision to represent each distinct integer and thus could cause problems. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 6 +++--- src/conf/node_device_conf.c | 4 ++-- src/conf/storage_conf.c | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7dba65cfeb..7984a15c46 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -18103,10 +18103,10 @@ virDomainDefClockParse(virDomainDef *def, break; case VIR_DOMAIN_CLOCK_OFFSET_VARIABLE: - if (virXPathLongLong("number(./clock/@adjustment)", ctxt, + if (virXPathLongLong("string(./clock/@adjustment)", ctxt, &def->clock.data.variable.adjustment) < 0) def->clock.data.variable.adjustment = 0; - if (virXPathLongLong("number(./clock/@adjustment0)", ctxt, + if (virXPathLongLong("string(./clock/@adjustment0)", ctxt, &def->clock.data.variable.adjustment0) < 0) def->clock.data.variable.adjustment0 = 0; tmp = virXPathString("string(./clock/@basis)", ctxt); @@ -18132,7 +18132,7 @@ virDomainDefClockParse(virDomainDef *def, break; case VIR_DOMAIN_CLOCK_OFFSET_ABSOLUTE: - if (virXPathULongLong("number(./clock/@start)", ctxt, + if (virXPathULongLong("string(./clock/@start)", ctxt, &def->clock.data.starttime) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing 'start' attribute for clock with offset='absolute'")); diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index b079c3713d..3bff17dae2 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -1463,7 +1463,7 @@ virNodeDevCapStorageParseXML(xmlXPathContextPtr ctxt, storage->media_label = virXPathString("string(./media_label[1])", ctxt); val = 0; - if (virNodeDevCapsDefParseULongLong("number(./media_size[1])", ctxt, &val, def, + if (virNodeDevCapsDefParseULongLong("string(./media_size[1])", ctxt, &val, def, _("no removable media size supplied for '%s'"), _("invalid removable media size supplied for '%s'")) < 0) { return -1; @@ -1481,7 +1481,7 @@ virNodeDevCapStorageParseXML(xmlXPathContextPtr ctxt, if (!(storage->flags & VIR_NODE_DEV_CAP_STORAGE_REMOVABLE)) { val = 0; - if (virNodeDevCapsDefParseULongLong("number(./size[1])", ctxt, &val, def, + if (virNodeDevCapsDefParseULongLong("string(./size[1])", ctxt, &val, def, _("no size supplied for '%s'"), _("invalid size supplied for '%s'")) < 0) return -1; diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 0f4fe1451e..b570679de2 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -720,7 +720,7 @@ virStorageDefParsePerms(xmlXPathContextPtr ctxt, perms->uid = (uid_t) -1; } else { /* We previously could output -1, so continue to parse it */ - if (virXPathLongLong("number(./owner)", ctxt, &val) < 0 || + if (virXPathLongLong("string(./owner)", ctxt, &val) < 0 || ((uid_t)val != val && val != -1)) { virReportError(VIR_ERR_XML_ERROR, "%s", @@ -735,7 +735,7 @@ virStorageDefParsePerms(xmlXPathContextPtr ctxt, perms->gid = (gid_t) -1; } else { /* We previously could output -1, so continue to parse it */ - if (virXPathLongLong("number(./group)", ctxt, &val) < 0 || + if (virXPathLongLong("string(./group)", ctxt, &val) < 0 || ((gid_t) val != val && val != -1)) { virReportError(VIR_ERR_XML_ERROR, "%s", -- 2.37.3
On a Monday in 2022, Peter Krempa wrote:
When the 'string()' conversion is used the number is parsed inside libvirt by our internal helpers which work on integers in contrast to when 'number()' is used and libxml2 uses a 'double' variable internally.
On the upper extremes of the 64 bit variables the doulbe precision
*double
variable doesn't have enough precision to represent each distinct integer and thus could cause problems.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 6 +++--- src/conf/node_device_conf.c | 4 ++-- src/conf/storage_conf.c | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Extract the internals of virXPathString which evaluate the XPath and validate that the returned object is a string into a new helper named 'virXPathEvalString'. The function will be later reused in the number XPath evaluation functions. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virxml.c | 39 ++++++++++++++++++++++++++++++--------- 1 file changed, 30 insertions(+), 9 deletions(-) diff --git a/src/util/virxml.c b/src/util/virxml.c index e771471dd7..aec475ccfd 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -60,6 +60,34 @@ virXMLXPathContextNew(xmlDocPtr xml) } +static xmlXPathObject * +virXPathEvalString(const char *xpath, + xmlXPathContextPtr ctxt) +{ + g_autoptr(xmlXPathObject) obj = NULL; + + if (!xpath) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Missing XPath expression")); + return NULL; + } + + if (!ctxt) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Missing XPath context")); + return NULL; + } + + if (!(obj = xmlXPathEval(BAD_CAST xpath, ctxt))) + return NULL; + + if (obj->type != XPATH_STRING || + !obj->stringval || + obj->stringval[0] == '\0') + return NULL; + + return g_steal_pointer(&obj); +} + + /** * virXPathString: * @xpath: the XPath string to evaluate @@ -76,16 +104,9 @@ virXPathString(const char *xpath, { g_autoptr(xmlXPathObject) obj = NULL; - if ((ctxt == NULL) || (xpath == NULL)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("Invalid parameter to virXPathString()")); + if (!(obj = virXPathEvalString(xpath, ctxt))) return NULL; - } - obj = xmlXPathEval(BAD_CAST xpath, ctxt); - if ((obj == NULL) || (obj->type != XPATH_STRING) || - (obj->stringval == NULL) || (obj->stringval[0] == 0)) { - return NULL; - } + return g_strdup((char *)obj->stringval); } -- 2.37.3
On a Monday in 2022, Peter Krempa wrote:
Extract the internals of virXPathString which evaluate the XPath and validate that the returned object is a string into a new helper named 'virXPathEvalString'.
The function will be later reused in the number XPath evaluation functions.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virxml.c | 39 ++++++++++++++++++++++++++++++--------- 1 file changed, 30 insertions(+), 9 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
The conversion from double is not precise enough at the extremes so it must not be used. Spell out that the callers are required to use a string() conversion in the XPath expression and remove the code path handling the direct conversion from numbers. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virxml.c | 60 ++++++++++++++--------------------------------- 1 file changed, 18 insertions(+), 42 deletions(-) diff --git a/src/util/virxml.c b/src/util/virxml.c index aec475ccfd..35c0340779 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -303,13 +303,15 @@ virXPathULongHex(const char *xpath, * virXPathULongLong: * @xpath: the XPath string to evaluate * @ctxt: an XPath context - * @value: the returned long long value + * @value: the returned unsigned long long value * - * Convenience function to evaluate an XPath number + * Convenience function to evaluate an XPath number. The @xpath expression + * must ensure that the evaluated value is returned as a string (use the + * 'string()' conversion in the expression). * * Returns 0 in case of success in which case @value is set, * or -1 if the XPath evaluation failed or -2 if the - * value doesn't have a long format. + * value doesn't have a unsigned long long format. */ int virXPathULongLong(const char *xpath, @@ -317,28 +319,14 @@ virXPathULongLong(const char *xpath, unsigned long long *value) { g_autoptr(xmlXPathObject) obj = NULL; - int ret = 0; - if ((ctxt == NULL) || (xpath == NULL) || (value == NULL)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("Invalid parameter to virXPathULong()")); + if (!(obj = virXPathEvalString(xpath, ctxt))) return -1; - } - obj = xmlXPathEval(BAD_CAST xpath, ctxt); - if ((obj != NULL) && (obj->type == XPATH_STRING) && - (obj->stringval != NULL) && (obj->stringval[0] != 0)) { - if (virStrToLong_ull((char *) obj->stringval, NULL, 10, value) < 0) - ret = -2; - } else if ((obj != NULL) && (obj->type == XPATH_NUMBER) && - (!(isnan(obj->floatval)))) { - *value = (unsigned long long) obj->floatval; - if (*value != obj->floatval) - ret = -2; - } else { - ret = -1; - } - return ret; + if (virStrToLong_ull((char *) obj->stringval, NULL, 10, value) < 0) + return -2; + + return 0; } @@ -348,7 +336,9 @@ virXPathULongLong(const char *xpath, * @ctxt: an XPath context * @value: the returned long long value * - * Convenience function to evaluate an XPath number + * Convenience function to evaluate an XPath number. The @xpath expression + * must ensure that the evaluated value is returned as a string (use the + * 'string()' conversion in the expression). * * Returns 0 in case of success in which case @value is set, * or -1 if the XPath evaluation failed or -2 if the @@ -360,28 +350,14 @@ virXPathLongLong(const char *xpath, long long *value) { g_autoptr(xmlXPathObject) obj = NULL; - int ret = 0; - if ((ctxt == NULL) || (xpath == NULL) || (value == NULL)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("Invalid parameter to virXPathLongLong()")); + if (!(obj = virXPathEvalString(xpath, ctxt))) return -1; - } - obj = xmlXPathEval(BAD_CAST xpath, ctxt); - if ((obj != NULL) && (obj->type == XPATH_STRING) && - (obj->stringval != NULL) && (obj->stringval[0] != 0)) { - if (virStrToLong_ll((char *) obj->stringval, NULL, 10, value) < 0) - ret = -2; - } else if ((obj != NULL) && (obj->type == XPATH_NUMBER) && - (!(isnan(obj->floatval)))) { - *value = (long long) obj->floatval; - if (*value != obj->floatval) - ret = -2; - } else { - ret = -1; - } - return ret; + if (virStrToLong_ll((char *) obj->stringval, NULL, 10, value) < 0) + return -2; + + return 0; } -- 2.37.3
On a Monday in 2022, Peter Krempa wrote:
The conversion from double is not precise enough at the extremes so it must not be used.
Spell out that the callers are required to use a string() conversion in the XPath expression and remove the code path handling the direct conversion from numbers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virxml.c | 60 ++++++++++++++--------------------------------- 1 file changed, 18 insertions(+), 42 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Passing negative number as an alias for the max value is an anti-feature we unfortunately allowed in virsh, but luckily never encouraged in the XML. Refuse numbers with negative sign when parsing unsigned long long from XPaths. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virxml.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virxml.c b/src/util/virxml.c index 35c0340779..067fef8856 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -323,7 +323,7 @@ virXPathULongLong(const char *xpath, if (!(obj = virXPathEvalString(xpath, ctxt))) return -1; - if (virStrToLong_ull((char *) obj->stringval, NULL, 10, value) < 0) + if (virStrToLong_ullp((char *) obj->stringval, NULL, 10, value) < 0) return -2; return 0; -- 2.37.3
On a Monday in 2022, Peter Krempa wrote:
Passing negative number as an alias for the max value is an anti-feature we unfortunately allowed in virsh, but luckily never encouraged in the XML.
Refuse numbers with negative sign when parsing unsigned long long from XPaths.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virxml.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Upcoming patches will require that the XML XPath query returns a string for conversion in virXPathUInt. Convert all the XPaths used with virNodeDevCapsDefParseUInt which uses virXPathUInt internally. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/node_device_conf.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 3bff17dae2..883f66f4d5 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -1502,25 +1502,25 @@ virNodeDevCapSCSIParseXML(xmlXPathContextPtr ctxt, ctxt->node = node; - if (virNodeDevCapsDefParseUInt("number(./host[1])", ctxt, + if (virNodeDevCapsDefParseUInt("string(./host[1])", ctxt, &scsi->host, def, _("no SCSI host ID supplied for '%s'"), _("invalid SCSI host ID supplied for '%s'")) < 0) return -1; - if (virNodeDevCapsDefParseUInt("number(./bus[1])", ctxt, + if (virNodeDevCapsDefParseUInt("string(./bus[1])", ctxt, &scsi->bus, def, _("no SCSI bus ID supplied for '%s'"), _("invalid SCSI bus ID supplied for '%s'")) < 0) return -1; - if (virNodeDevCapsDefParseUInt("number(./target[1])", ctxt, + if (virNodeDevCapsDefParseUInt("string(./target[1])", ctxt, &scsi->target, def, _("no SCSI target ID supplied for '%s'"), _("invalid SCSI target ID supplied for '%s'")) < 0) return -1; - if (virNodeDevCapsDefParseUInt("number(./lun[1])", ctxt, + if (virNodeDevCapsDefParseUInt("string(./lun[1])", ctxt, &scsi->lun, def, _("no SCSI LUN ID supplied for '%s'"), _("invalid SCSI LUN ID supplied for '%s'")) < 0) @@ -1615,7 +1615,7 @@ virNodeDevCapSCSIHostParseXML(xmlXPathContextPtr ctxt, ctxt->node = node; if (create == EXISTING_DEVICE) { - if (virNodeDevCapsDefParseUInt("number(./host[1])", ctxt, + if (virNodeDevCapsDefParseUInt("string(./host[1])", ctxt, &scsi_host->host, def, _("no SCSI host ID supplied for '%s'"), _("invalid SCSI host ID supplied for '%s'")) < 0) { @@ -1775,25 +1775,25 @@ virNodeDevCapUSBInterfaceParseXML(xmlXPathContextPtr ctxt, ctxt->node = node; - if (virNodeDevCapsDefParseUInt("number(./number[1])", ctxt, + if (virNodeDevCapsDefParseUInt("string(./number[1])", ctxt, &usb_if->number, def, _("no USB interface number supplied for '%s'"), _("invalid USB interface number supplied for '%s'")) < 0) return -1; - if (virNodeDevCapsDefParseUInt("number(./class[1])", ctxt, + if (virNodeDevCapsDefParseUInt("string(./class[1])", ctxt, &usb_if->klass, def, _("no USB interface class supplied for '%s'"), _("invalid USB interface class supplied for '%s'")) < 0) return -1; - if (virNodeDevCapsDefParseUInt("number(./subclass[1])", ctxt, + if (virNodeDevCapsDefParseUInt("string(./subclass[1])", ctxt, &usb_if->subclass, def, _("no USB interface subclass supplied for '%s'"), _("invalid USB interface subclass supplied for '%s'")) < 0) return -1; - if (virNodeDevCapsDefParseUInt("number(./protocol[1])", ctxt, + if (virNodeDevCapsDefParseUInt("string(./protocol[1])", ctxt, &usb_if->protocol, def, _("no USB interface protocol supplied for '%s'"), _("invalid USB interface protocol supplied for '%s'")) < 0) @@ -1839,13 +1839,13 @@ virNodeDevCapUSBDevParseXML(xmlXPathContextPtr ctxt, ctxt->node = node; - if (virNodeDevCapsDefParseUInt("number(./bus[1])", ctxt, + if (virNodeDevCapsDefParseUInt("string(./bus[1])", ctxt, &usb_dev->bus, def, _("no USB bus number supplied for '%s'"), _("invalid USB bus number supplied for '%s'")) < 0) return -1; - if (virNodeDevCapsDefParseUInt("number(./device[1])", ctxt, + if (virNodeDevCapsDefParseUInt("string(./device[1])", ctxt, &usb_dev->device, def, _("no USB device number supplied for '%s'"), _("invalid USB device number supplied for '%s'")) < 0) @@ -2084,25 +2084,25 @@ virNodeDevCapPCIDevParseXML(xmlXPathContextPtr ctxt, pci_dev->klass = -1; } - if (virNodeDevCapsDefParseUInt("number(./domain[1])", ctxt, + if (virNodeDevCapsDefParseUInt("string(./domain[1])", ctxt, &pci_dev->domain, def, _("no PCI domain ID supplied for '%s'"), _("invalid PCI domain ID supplied for '%s'")) < 0) goto out; - if (virNodeDevCapsDefParseUInt("number(./bus[1])", ctxt, + if (virNodeDevCapsDefParseUInt("string(./bus[1])", ctxt, &pci_dev->bus, def, _("no PCI bus ID supplied for '%s'"), _("invalid PCI bus ID supplied for '%s'")) < 0) goto out; - if (virNodeDevCapsDefParseUInt("number(./slot[1])", ctxt, + if (virNodeDevCapsDefParseUInt("string(./slot[1])", ctxt, &pci_dev->slot, def, _("no PCI slot ID supplied for '%s'"), _("invalid PCI slot ID supplied for '%s'")) < 0) goto out; - if (virNodeDevCapsDefParseUInt("number(./function[1])", ctxt, + if (virNodeDevCapsDefParseUInt("string(./function[1])", ctxt, &pci_dev->function, def, _("no PCI function ID supplied for '%s'"), _("invalid PCI function ID supplied for '%s'")) < 0) -- 2.37.3
On a Monday in 2022, Peter Krempa wrote:
Upcoming patches will require that the XML XPath query returns a string for conversion in virXPathUInt. Convert all the XPaths used with virNodeDevCapsDefParseUInt which uses virXPathUInt internally.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/node_device_conf.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Convert the rest of the XPath expressions used with virXPathUInt directly to convert via string(). This will become mandatory in upcoming patches. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/node_device_conf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 883f66f4d5..1e798651c5 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -942,7 +942,7 @@ virNodeDevCapMdevTypesParseXML(xmlXPathContextPtr ctxt, goto cleanup; } - if (virXPathUInt("number(./availableInstances)", ctxt, + if (virXPathUInt("string(./availableInstances)", ctxt, &type->available_instances) < 0) { virReportError(VIR_ERR_XML_ERROR, _("missing number of available instances for " @@ -2257,7 +2257,7 @@ virNodeDevCapMdevParseXML(xmlXPathContextPtr ctxt, /* 'iommuGroup' is optional, only report an error if the supplied value is * invalid (-2), not if it's missing (-1) */ - if (virXPathUInt("number(./iommuGroup[1]/@number)", + if (virXPathUInt("string(./iommuGroup[1]/@number)", ctxt, &mdev->iommuGroupNumber) < -1) { virReportError(VIR_ERR_INTERNAL_ERROR, _("invalid iommuGroup number attribute for '%s'"), -- 2.37.3
On a Monday in 2022, Peter Krempa wrote:
Convert the rest of the XPath expressions used with virXPathUInt directly to convert via string(). This will become mandatory in upcoming patches.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/node_device_conf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Upcoming patches will require that the XML XPath query returns a string for conversion in virXPathInt. Convert all the XPaths used with virNodeDevCapsDefParseIntOptional which uses virXPathInt internally. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/node_device_conf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 1e798651c5..9d04c5fdf4 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -1623,7 +1623,7 @@ virNodeDevCapSCSIHostParseXML(xmlXPathContextPtr ctxt, } /* Optional unique_id value */ scsi_host->unique_id = -1; - if (virNodeDevCapsDefParseIntOptional("number(./unique_id[1])", ctxt, + if (virNodeDevCapsDefParseIntOptional("string(./unique_id[1])", ctxt, &scsi_host->unique_id, def, _("invalid unique_id supplied for '%s'")) < 0) { return -1; @@ -2140,7 +2140,7 @@ virNodeDevCapPCIDevParseXML(xmlXPathContextPtr ctxt, /* The default value is -1 since zero is valid NUMA node number */ pci_dev->numa_node = -1; - if (virNodeDevCapsDefParseIntOptional("number(./numa[1]/@node)", ctxt, + if (virNodeDevCapsDefParseIntOptional("string(./numa[1]/@node)", ctxt, &pci_dev->numa_node, def, _("invalid NUMA node ID supplied for '%s'")) < 0) goto out; -- 2.37.3
On a Monday in 2022, Peter Krempa wrote:
Upcoming patches will require that the XML XPath query returns a string for conversion in virXPathInt. Convert all the XPaths used with virNodeDevCapsDefParseIntOptional which uses virXPathInt internally.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/node_device_conf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
The code only wants to refuse cases where more than one 'numatune' element is present which can be achieved by using 'virXPathBoolean'. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/numa_conf.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c index 390ef49b84..688aa7b409 100644 --- a/src/conf/numa_conf.c +++ b/src/conf/numa_conf.c @@ -226,18 +226,13 @@ virDomainNumatuneParseXML(virDomainNuma *numa, { g_autofree char *modestr = NULL; int mode = -1; - int n = 0; g_autofree char *placementstr = NULL; int placement = -1; g_autofree char *nodesetstr = NULL; g_autoptr(virBitmap) nodeset = NULL; xmlNodePtr node = NULL; - if (virXPathInt("count(./numatune)", ctxt, &n) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("cannot extract numatune nodes")); - return -1; - } else if (n > 1) { + if (virXPathBoolean("count(./numatune) > 1", ctxt) == 1) { virReportError(VIR_ERR_XML_ERROR, "%s", _("only one numatune is supported")); return -1; -- 2.37.3
On a Monday in 2022, Peter Krempa wrote:
The code only wants to refuse cases where more than one 'numatune' element is present which can be achieved by using 'virXPathBoolean'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/numa_conf.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c index 390ef49b84..688aa7b409 100644 --- a/src/conf/numa_conf.c +++ b/src/conf/numa_conf.c @@ -226,18 +226,13 @@ virDomainNumatuneParseXML(virDomainNuma *numa, { g_autofree char *modestr = NULL; int mode = -1; - int n = 0; g_autofree char *placementstr = NULL; int placement = -1; g_autofree char *nodesetstr = NULL; g_autoptr(virBitmap) nodeset = NULL; xmlNodePtr node = NULL;
- if (virXPathInt("count(./numatune)", ctxt, &n) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("cannot extract numatune nodes")); - return -1; - } else if (n > 1) { + if (virXPathBoolean("count(./numatune) > 1", ctxt) == 1) {
Alternatively, "boolean(./numatune[1])" Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Move the parser into a helper function named 'virCPUDefParseXMLCache' and use the virXMLProp* helpers instead of multiple XPath lookups. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/cpu_conf.c | 58 ++++++++++++++++++++++++++------------------- 1 file changed, 34 insertions(+), 24 deletions(-) diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index fdbb564cc8..a33f39ef31 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -312,6 +312,34 @@ virCPUDefParseXMLString(const char *xml, } +static int +virCPUDefParseXMLCache(virCPUDef *def, + xmlNodePtr node) +{ + int rc; + + def->cache = g_new0(virCPUCacheDef, 1); + + if ((rc = virXMLPropInt(node, "level", 10, VIR_XML_PROP_NONNEGATIVE, + &def->cache->level, -1)) < 0) + return -1; + + if (rc == 1) { + if (def->cache->level < 1 || def->cache->level > 3) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("invalid CPU cache level, must be in range [1,3]")); + return -1; + } + } + + if (virXMLPropEnum(node, "mode", virCPUCacheModeTypeFromString, + VIR_XML_PROP_REQUIRED, &def->cache->mode) < 0) + return -1; + + return 0; +} + + /* * Parses CPU definition XML from a node pointed to by @xpath. If @xpath is * NULL, the current node of @ctxt is used (i.e., it is a shortcut to "."). @@ -335,6 +363,8 @@ virCPUDefParseXML(xmlXPathContextPtr ctxt, g_autofree xmlNodePtr *nodes = NULL; xmlNodePtr topology = NULL; xmlNodePtr maxphysaddrNode = NULL; + g_autofree xmlNodePtr *cacheNodes = NULL; + ssize_t ncacheNodes = 0; VIR_XPATH_NODE_AUTORESTORE(ctxt) int n; int rv; @@ -626,35 +656,15 @@ virCPUDefParseXML(xmlXPathContextPtr ctxt, def->features[i].policy = policy; } - if (virXPathInt("count(./cache)", ctxt, &n) < 0) { - return -1; - } else if (n > 1) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("at most one CPU cache element may be specified")); - return -1; - } else if (n == 1) { - int level = -1; - g_autofree char *strmode = NULL; - int mode; - - if (virXPathBoolean("boolean(./cache[1]/@level)", ctxt) == 1 && - (virXPathInt("string(./cache[1]/@level)", ctxt, &level) < 0 || - level < 1 || level > 3)) { + if ((ncacheNodes = virXPathNodeSet("./cache", ctxt, &cacheNodes)) > 0) { + if (ncacheNodes > 1) { virReportError(VIR_ERR_XML_ERROR, "%s", - _("invalid CPU cache level, must be in range [1,3]")); + _("at most one CPU cache element may be specified")); return -1; } - if (!(strmode = virXPathString("string(./cache[1]/@mode)", ctxt)) || - (mode = virCPUCacheModeTypeFromString(strmode)) < 0) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("missing or invalid CPU cache mode")); + if (virCPUDefParseXMLCache(def, cacheNodes[0]) < 0) return -1; - } - - def->cache = g_new0(virCPUCacheDef, 1); - def->cache->level = level; - def->cache->mode = mode; } if ((maxphysaddrNode = virXPathNode("./maxphysaddr[1]", ctxt))) { -- 2.37.3
On a Monday in 2022, Peter Krempa wrote:
Move the parser into a helper function named 'virCPUDefParseXMLCache' and use the virXMLProp* helpers instead of multiple XPath lookups.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/cpu_conf.c | 58 ++++++++++++++++++++++++++------------------- 1 file changed, 34 insertions(+), 24 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Similarly to the refactor of virXPath(U)LongLong drop the ability to convert from the internal double value forcing the use of the 'string()' conversion. In case of 32 bit integers there's no problem with overflows, but we can implement the code identically to what we have in the other helpers. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virxml.c | 38 ++++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/src/util/virxml.c b/src/util/virxml.c index 067fef8856..d885fc1680 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -149,7 +149,9 @@ virXPathLongBase(const char *xpath, * @ctxt: an XPath context * @value: the returned int value * - * Convenience function to evaluate an XPath number + * Convenience function to evaluate an XPath number. The @xpath expression + * must ensure that the evaluated value is returned as a string (use the + * 'string()' conversion in the expression). * * Returns 0 in case of success in which case @value is set, * or -1 if the XPath evaluation failed or -2 if the @@ -160,15 +162,14 @@ virXPathInt(const char *xpath, xmlXPathContextPtr ctxt, int *value) { - long tmp; - int ret; + g_autoptr(xmlXPathObject) obj = NULL; - ret = virXPathLongBase(xpath, ctxt, 10, &tmp); - if (ret < 0) - return ret; - if ((int) tmp != tmp) + if (!(obj = virXPathEvalString(xpath, ctxt))) + return -1; + + if (virStrToLong_i((char *) obj->stringval, NULL, 10, value) < 0) return -2; - *value = tmp; + return 0; } @@ -230,28 +231,29 @@ virXPathULongBase(const char *xpath, * virXPathUInt: * @xpath: the XPath string to evaluate * @ctxt: an XPath context - * @value: the returned int value + * @value: the returned unsigned int value * - * Convenience function to evaluate an XPath number + * Convenience function to evaluate an XPath number. The @xpath expression + * must ensure that the evaluated value is returned as a string (use the + * 'string()' conversion in the expression). * * Returns 0 in case of success in which case @value is set, * or -1 if the XPath evaluation failed or -2 if the - * value doesn't have an int format. + * value doesn't have an unsigned int format. */ int virXPathUInt(const char *xpath, xmlXPathContextPtr ctxt, unsigned int *value) { - unsigned long tmp; - int ret; + g_autoptr(xmlXPathObject) obj = NULL; - ret = virXPathULongBase(xpath, ctxt, 10, &tmp); - if (ret < 0) - return ret; - if ((unsigned int) tmp != tmp) + if (!(obj = virXPathEvalString(xpath, ctxt))) + return -1; + + if (virStrToLong_ui((char *) obj->stringval, NULL, 10, value) < 0) return -2; - *value = tmp; + return 0; } -- 2.37.3
On a Monday in 2022, Peter Krempa wrote:
Similarly to the refactor of virXPath(U)LongLong drop the ability to convert from the internal double value forcing the use of the 'string()' conversion.
In case of 32 bit integers there's no problem with overflows, but we can implement the code identically to what we have in the other helpers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virxml.c | 38 ++++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
In an effort to remove the 'Long' variants of XPath number fetching functions we need a way to replace the hex number parsing capability. The new helpers are created from the originals by adding a 'base' argument and keeping the original function as a wrapper to pass 10. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt_private.syms | 2 ++ src/util/virxml.c | 42 ++++++++++++++++++++++++++++++---------- src/util/virxml.h | 10 ++++++++++ 3 files changed, 44 insertions(+), 10 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c6445dedcb..dab3b389c8 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3699,9 +3699,11 @@ virXPathNode; virXPathNodeSet; virXPathString; virXPathUInt; +virXPathUIntBase; virXPathULong; virXPathULongHex; virXPathULongLong; +virXPathULongLongBase; # Let emacs know we want case-insensitive sorting diff --git a/src/util/virxml.c b/src/util/virxml.c index d885fc1680..a47a5d49fc 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -228,9 +228,10 @@ virXPathULongBase(const char *xpath, /** - * virXPathUInt: + * virXPathUIntBase: * @xpath: the XPath string to evaluate * @ctxt: an XPath context + * @base: base of the number to fetch @value as * @value: the returned unsigned int value * * Convenience function to evaluate an XPath number. The @xpath expression @@ -242,22 +243,32 @@ virXPathULongBase(const char *xpath, * value doesn't have an unsigned int format. */ int -virXPathUInt(const char *xpath, - xmlXPathContextPtr ctxt, - unsigned int *value) +virXPathUIntBase(const char *xpath, + xmlXPathContextPtr ctxt, + unsigned int base, + unsigned int *value) { g_autoptr(xmlXPathObject) obj = NULL; if (!(obj = virXPathEvalString(xpath, ctxt))) return -1; - if (virStrToLong_ui((char *) obj->stringval, NULL, 10, value) < 0) + if (virStrToLong_ui((char *) obj->stringval, NULL, base, value) < 0) return -2; return 0; } +int +virXPathUInt(const char *xpath, + xmlXPathContextPtr ctxt, + unsigned int *value) +{ + return virXPathUIntBase(xpath, ctxt, 10, value); +} + + /** * virXPathULong: * @xpath: the XPath string to evaluate @@ -302,9 +313,10 @@ virXPathULongHex(const char *xpath, /** - * virXPathULongLong: + * virXPathULongLongBase: * @xpath: the XPath string to evaluate * @ctxt: an XPath context + * @base: base of the number to fetch @value as * @value: the returned unsigned long long value * * Convenience function to evaluate an XPath number. The @xpath expression @@ -316,22 +328,32 @@ virXPathULongHex(const char *xpath, * value doesn't have a unsigned long long format. */ int -virXPathULongLong(const char *xpath, - xmlXPathContextPtr ctxt, - unsigned long long *value) +virXPathULongLongBase(const char *xpath, + xmlXPathContextPtr ctxt, + unsigned int base, + unsigned long long *value) { g_autoptr(xmlXPathObject) obj = NULL; if (!(obj = virXPathEvalString(xpath, ctxt))) return -1; - if (virStrToLong_ullp((char *) obj->stringval, NULL, 10, value) < 0) + if (virStrToLong_ullp((char *) obj->stringval, NULL, base, value) < 0) return -2; return 0; } +int +virXPathULongLong(const char *xpath, + xmlXPathContextPtr ctxt, + unsigned long long *value) +{ + return virXPathULongLongBase(xpath, ctxt, 10, value); +} + + /** * virXPathLongLong: * @xpath: the XPath string to evaluate diff --git a/src/util/virxml.h b/src/util/virxml.h index fe07e2d223..bdd2e9145d 100644 --- a/src/util/virxml.h +++ b/src/util/virxml.h @@ -51,6 +51,11 @@ virXPathInt(const char *xpath, xmlXPathContextPtr ctxt, int *value); int +virXPathUIntBase(const char *xpath, + xmlXPathContextPtr ctxt, + unsigned int base, + unsigned int *value); +int virXPathUInt(const char *xpath, xmlXPathContextPtr ctxt, unsigned int *value); @@ -63,6 +68,11 @@ virXPathULong(const char *xpath, xmlXPathContextPtr ctxt, unsigned long *value); int +virXPathULongLongBase(const char *xpath, + xmlXPathContextPtr ctxt, + unsigned int base, + unsigned long long *value); +int virXPathULongLong(const char *xpath, xmlXPathContextPtr ctxt, unsigned long long *value); -- 2.37.3
On a Monday in 2022, Peter Krempa wrote:
In an effort to remove the 'Long' variants of XPath number fetching functions we need a way to replace the hex number parsing capability.
The new helpers are created from the originals by adding a 'base' argument and keeping the original function as a wrapper to pass 10.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt_private.syms | 2 ++ src/util/virxml.c | 42 ++++++++++++++++++++++++++++++---------- src/util/virxml.h | 10 ++++++++++ 3 files changed, 44 insertions(+), 10 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Switch to the proper function for parsing integer variant of a hex number via XPath and spell out properly that the argument is 'unsigned int'. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/node_device_conf.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 9d04c5fdf4..ccedf77443 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -1808,15 +1808,14 @@ virNodeDevCapUSBInterfaceParseXML(xmlXPathContextPtr ctxt, static int virNodeDevCapsDefParseHexId(const char *xpath, xmlXPathContextPtr ctxt, - unsigned *value, + unsigned int *value, virNodeDeviceDef *def, const char *missing_error_fmt, const char *invalid_error_fmt) { int ret; - unsigned long val; - ret = virXPathULongHex(xpath, ctxt, &val); + ret = virXPathUIntBase(xpath, ctxt, 16, value); if (ret < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, ret == -1 ? missing_error_fmt : invalid_error_fmt, @@ -1824,7 +1823,6 @@ virNodeDevCapsDefParseHexId(const char *xpath, return -1; } - *value = val; return 0; } -- 2.37.3
On a Monday in 2022, Peter Krempa wrote:
Switch to the proper function for parsing integer variant of a hex number via XPath and spell out properly that the argument is 'unsigned int'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/node_device_conf.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Passing negative number as an alias for the max value is an anti-feature we unfortunately allowed in virsh, but luckily never encouraged in the XML. Refuse numbers with negative sign when parsing unsigned int from XPaths. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virxml.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virxml.c b/src/util/virxml.c index a47a5d49fc..d80a69f163 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -253,7 +253,7 @@ virXPathUIntBase(const char *xpath, if (!(obj = virXPathEvalString(xpath, ctxt))) return -1; - if (virStrToLong_ui((char *) obj->stringval, NULL, base, value) < 0) + if (virStrToLong_uip((char *) obj->stringval, NULL, base, value) < 0) return -2; return 0; -- 2.37.3
On a Monday in 2022, Peter Krempa wrote:
Passing negative number as an alias for the max value is an anti-feature we unfortunately allowed in virsh, but luckily never encouraged in the XML.
Refuse numbers with negative sign when parsing unsigned int from XPaths.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virxml.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Use the function for appropriate types and simplify the error logic. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/test/test_driver.c | 51 +++++++++++++++++------------------------- 1 file changed, 20 insertions(+), 31 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 67c70de11d..5a988fbcbd 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -874,41 +874,30 @@ testParseXMLDocFromFile(xmlNodePtr node, static int testParseNodeInfo(virNodeInfoPtr nodeInfo, xmlXPathContextPtr ctxt) { - long l; - int ret; g_autofree char *str = NULL; + unsigned int activeCpus; + unsigned long long memory = 0; + int rc; - ret = virXPathLong("string(/node/cpu/nodes[1])", ctxt, &l); - if (ret == 0) { - nodeInfo->nodes = l; - } else if (ret == -2) { + if (virXPathUInt("string(/node/cpu/nodes[1])", ctxt, &nodeInfo->nodes) == -2) { virReportError(VIR_ERR_XML_ERROR, "%s", _("invalid node cpu nodes value")); return -1; } - ret = virXPathLong("string(/node/cpu/sockets[1])", ctxt, &l); - if (ret == 0) { - nodeInfo->sockets = l; - } else if (ret == -2) { + if (virXPathUInt("string(/node/cpu/sockets[1])", ctxt, &nodeInfo->sockets) == -2) { virReportError(VIR_ERR_XML_ERROR, "%s", _("invalid node cpu sockets value")); return -1; } - ret = virXPathLong("string(/node/cpu/cores[1])", ctxt, &l); - if (ret == 0) { - nodeInfo->cores = l; - } else if (ret == -2) { + if (virXPathUInt("string(/node/cpu/cores[1])", ctxt, &nodeInfo->cores) == -2) { virReportError(VIR_ERR_XML_ERROR, "%s", _("invalid node cpu cores value")); return -1; } - ret = virXPathLong("string(/node/cpu/threads[1])", ctxt, &l); - if (ret == 0) { - nodeInfo->threads = l; - } else if (ret == -2) { + if (virXPathUInt("string(/node/cpu/threads[1])", ctxt, &nodeInfo->threads) == -2) { virReportError(VIR_ERR_XML_ERROR, "%s", _("invalid node cpu threads value")); return -1; @@ -916,19 +905,19 @@ testParseNodeInfo(virNodeInfoPtr nodeInfo, xmlXPathContextPtr ctxt) nodeInfo->cpus = (nodeInfo->cores * nodeInfo->threads * nodeInfo->sockets * nodeInfo->nodes); - ret = virXPathLong("string(/node/cpu/active[1])", ctxt, &l); - if (ret == 0) { - if (l < nodeInfo->cpus) - nodeInfo->cpus = l; - } else if (ret == -2) { + + if ((rc = virXPathUInt("string(/node/cpu/active[1])", ctxt, &activeCpus)) == -2) { virReportError(VIR_ERR_XML_ERROR, "%s", _("invalid node cpu active value")); return -1; } - ret = virXPathLong("string(/node/cpu/mhz[1])", ctxt, &l); - if (ret == 0) { - nodeInfo->mhz = l; - } else if (ret == -2) { + + if (rc == 0) { + if (activeCpus < nodeInfo->cpus) + nodeInfo->cpus = activeCpus; + } + + if (virXPathUInt("string(/node/cpu/mhz[1])", ctxt, &nodeInfo->mhz) == -2) { virReportError(VIR_ERR_XML_ERROR, "%s", _("invalid node cpu mhz value")); return -1; @@ -943,15 +932,15 @@ testParseNodeInfo(virNodeInfoPtr nodeInfo, xmlXPathContextPtr ctxt) } } - ret = virXPathLong("string(/node/memory[1])", ctxt, &l); - if (ret == 0) { - nodeInfo->memory = l; - } else if (ret == -2) { + if ((rc = virXPathULongLong("string(/node/memory[1])", ctxt, &memory)) == -2) { virReportError(VIR_ERR_XML_ERROR, "%s", _("invalid node memory value")); return -1; } + if (rc == 0) + nodeInfo->memory = memory; + return 0; } -- 2.37.3
On a Monday in 2022, Peter Krempa wrote:
Use the function for appropriate types and simplify the error logic.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/test/test_driver.c | 51 +++++++++++++++++------------------------- 1 file changed, 20 insertions(+), 31 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Convert the two uses of virXPathLong to proper virXMLPropInt/virXMLPropLongLong so that virXPathLong can be removed in an upcoming patch. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7984a15c46..8be37a4040 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -17420,13 +17420,15 @@ virDomainDefParseIDs(virDomainDef *def, { g_autofree xmlNodePtr *nodes = NULL; g_autofree char *tmp = NULL; - long id = -1; int n; - if (!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE)) - if (virXPathLong("string(./@id)", ctxt, &id) < 0) - id = -1; - def->id = (int)id; + def->id = -1; + + if (!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE)) { + if (virXMLPropInt(ctxt->node, "id", 10, VIR_XML_PROP_NONNEGATIVE, + &def->id, -1) < 0) + return -1; + } /* Extract domain name */ if (!(def->name = virXPathString("string(./name[1])", ctxt))) { @@ -18983,7 +18985,7 @@ virDomainObjParseXML(xmlXPathContextPtr ctxt, virDomainXMLOption *xmlopt, unsigned int flags) { - long val; + long long vmpid; xmlNodePtr config; xmlNodePtr oldnode; g_autoptr(virDomainObj) obj = NULL; @@ -19026,12 +19028,11 @@ virDomainObjParseXML(xmlXPathContextPtr ctxt, virDomainObjSetState(obj, state, reason); - if (virXPathLong("string(./@pid)", ctxt, &val) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("invalid pid")); + if (virXMLPropLongLong(ctxt->node, "pid", 10, VIR_XML_PROP_REQUIRED, + &vmpid, 0) < 0) return NULL; - } - obj->pid = (pid_t)val; + + obj->pid = (pid_t) vmpid; if ((n = virXPathNodeSet("./taint", ctxt, &taintNodes)) < 0) return NULL; -- 2.37.3
On a Monday in 2022, Peter Krempa wrote:
Convert the two uses of virXPathLong to proper virXMLPropInt/virXMLPropLongLong so that virXPathLong can be removed in an upcoming patch.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
The function is now unused and we no longer want to promote use of the 'long' type. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt_private.syms | 1 - src/util/virxml.c | 53 ---------------------------------------- src/util/virxml.h | 4 --- 3 files changed, 58 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index dab3b389c8..0fc5481a26 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3693,7 +3693,6 @@ virXMLValidatorValidate; virXPathBoolean; virXPathContextNodeRestore; virXPathInt; -virXPathLong; virXPathLongLong; virXPathNode; virXPathNodeSet; diff --git a/src/util/virxml.c b/src/util/virxml.c index d80a69f163..f5ee9284a6 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -111,38 +111,6 @@ virXPathString(const char *xpath, } -static int -virXPathLongBase(const char *xpath, - xmlXPathContextPtr ctxt, - int base, - long *value) -{ - g_autoptr(xmlXPathObject) obj = NULL; - int ret = 0; - - if ((ctxt == NULL) || (xpath == NULL) || (value == NULL)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("Invalid parameter to virXPathLong()")); - return -1; - } - obj = xmlXPathEval(BAD_CAST xpath, ctxt); - if ((obj != NULL) && (obj->type == XPATH_STRING) && - (obj->stringval != NULL) && (obj->stringval[0] != 0)) { - if (virStrToLong_l((char *) obj->stringval, NULL, base, value) < 0) - ret = -2; - } else if ((obj != NULL) && (obj->type == XPATH_NUMBER) && - (!(isnan(obj->floatval)))) { - *value = (long) obj->floatval; - if (*value != obj->floatval) - ret = -2; - } else { - ret = -1; - } - - return ret; -} - - /** * virXPathInt: * @xpath: the XPath string to evaluate @@ -174,27 +142,6 @@ virXPathInt(const char *xpath, } -/** - * virXPathLong: - * @xpath: the XPath string to evaluate - * @ctxt: an XPath context - * @value: the returned long value - * - * Convenience function to evaluate an XPath number - * - * Returns 0 in case of success in which case @value is set, - * or -1 if the XPath evaluation failed or -2 if the - * value doesn't have a long format. - */ -int -virXPathLong(const char *xpath, - xmlXPathContextPtr ctxt, - long *value) -{ - return virXPathLongBase(xpath, ctxt, 10, value); -} - - static int virXPathULongBase(const char *xpath, xmlXPathContextPtr ctxt, diff --git a/src/util/virxml.h b/src/util/virxml.h index bdd2e9145d..439a2a9991 100644 --- a/src/util/virxml.h +++ b/src/util/virxml.h @@ -60,10 +60,6 @@ virXPathUInt(const char *xpath, xmlXPathContextPtr ctxt, unsigned int *value); int -virXPathLong(const char *xpath, - xmlXPathContextPtr ctxt, - long *value); -int virXPathULong(const char *xpath, xmlXPathContextPtr ctxt, unsigned long *value); -- 2.37.3
On a Monday in 2022, Peter Krempa wrote:
The function is now unused and we no longer want to promote use of the 'long' type.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt_private.syms | 1 - src/util/virxml.c | 53 ---------------------------------------- src/util/virxml.h | 4 --- 3 files changed, 58 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
The libvirt version is stored in an 'unsigned int' use the proper XPath query function for the type and remove the temporary variable. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_capabilities.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index dd50f85c73..63590f139f 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -4385,7 +4385,6 @@ virQEMUCapsLoadCache(virArch hostArch, g_autoptr(xmlDoc) doc = NULL; g_autoptr(xmlXPathContext) ctxt = NULL; long long int l; - unsigned long lu; if (!(doc = virXMLParse(filename, NULL, NULL, "qemuCaps", &ctxt, NULL, false))) return -1; @@ -4397,9 +4396,9 @@ virQEMUCapsLoadCache(virArch hostArch, } qemuCaps->libvirtCtime = (time_t)l; - qemuCaps->libvirtVersion = 0; - if (virXPathULong("string(./selfvers)", ctxt, &lu) == 0) - qemuCaps->libvirtVersion = lu; + if (virXMLPropUInt(ctxt->node, "selfvers", 10, VIR_XML_PROP_NONE, + &qemuCaps->libvirtVersion) < 0) + return -1; if (!skipInvalidation && (qemuCaps->libvirtCtime != virGetSelfLastChanged() || -- 2.37.3
On a Monday in 2022, Peter Krempa wrote:
The libvirt version is stored in an 'unsigned int' use the proper XPath query function for the type and remove the temporary variable.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_capabilities.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
The function extracts multiple attributes form a single element. Modify the function to stop using multiple XPath lookups. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 2 +- src/conf/network_conf.c | 4 +-- src/conf/networkcommon_conf.c | 61 +++++++++-------------------------- src/conf/networkcommon_conf.h | 3 +- 4 files changed, 19 insertions(+), 51 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 8be37a4040..cdf05e8fae 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6263,7 +6263,7 @@ virDomainNetIPInfoParseXML(const char *source, for (i = 0; i < nrouteNodes; i++) { virNetDevIPRoute *route = NULL; - if (!(route = virNetDevIPRouteParseXML(source, routeNodes[i], ctxt))) + if (!(route = virNetDevIPRouteParseXML(source, routeNodes[i]))) goto error; VIR_APPEND_ELEMENT(def->routes, def->nroutes, route); diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index be43894050..44ac5408d1 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -1824,9 +1824,7 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt, for (i = 0; i < nRoutes; i++) { virNetDevIPRoute *route = NULL; - if (!(route = virNetDevIPRouteParseXML(def->name, - routeNodes[i], - ctxt))) + if (!(route = virNetDevIPRouteParseXML(def->name, routeNodes[i]))) return NULL; def->routes[i] = route; def->nroutes++; diff --git a/src/conf/networkcommon_conf.c b/src/conf/networkcommon_conf.c index 7bfcd2e9b4..275847853d 100644 --- a/src/conf/networkcommon_conf.c +++ b/src/conf/networkcommon_conf.c @@ -211,58 +211,29 @@ virNetDevIPRouteCreate(const char *errorDetail, virNetDevIPRoute * virNetDevIPRouteParseXML(const char *errorDetail, - xmlNodePtr node, - xmlXPathContextPtr ctxt) + xmlNodePtr node) { - /* - * virNetDevIPRoute object is already allocated as part - * of an array. On failure clear: it out, but don't free it. - */ - - VIR_XPATH_NODE_AUTORESTORE(ctxt) - g_autofree char *family = NULL; - g_autofree char *address = NULL; - g_autofree char *netmask = NULL; - g_autofree char *gateway = NULL; - unsigned long prefix = 0, metric = 0; - int prefixRc, metricRc; + g_autofree char *family = virXMLPropString(node, "family"); + g_autofree char *address = virXMLPropString(node, "address"); + g_autofree char *netmask = virXMLPropString(node, "netmask"); + g_autofree char *gateway = virXMLPropString(node, "gateway"); + unsigned int prefix = 0; + unsigned int metric = 0; bool hasPrefix = false; bool hasMetric = false; + int rc; - ctxt->node = node; - - /* grab raw data from XML */ - family = virXPathString("string(./@family)", ctxt); - address = virXPathString("string(./@address)", ctxt); - netmask = virXPathString("string(./@netmask)", ctxt); - gateway = virXPathString("string(./@gateway)", ctxt); - prefixRc = virXPathULong("string(./@prefix)", ctxt, &prefix); - if (prefixRc == -2) { - virReportError(VIR_ERR_XML_ERROR, - _("%s: Invalid prefix specified " - "in route definition"), - errorDetail); + if ((rc = virXMLPropUInt(node, "prefix", 10, VIR_XML_PROP_NONE, &prefix)) < 0) return NULL; - } - hasPrefix = (prefixRc == 0); - metricRc = virXPathULong("string(./@metric)", ctxt, &metric); - if (metricRc == -2) { - virReportError(VIR_ERR_XML_ERROR, - _("%s: Invalid metric specified " - "in route definition"), - errorDetail); + + if (rc == 1) + hasPrefix = true; + + if ((rc = virXMLPropUInt(node, "metric", 10, VIR_XML_PROP_NONZERO, &metric)) < 0) return NULL; - } - if (metricRc == 0) { + + if (rc == 1) hasMetric = true; - if (metric == 0) { - virReportError(VIR_ERR_XML_ERROR, - _("%s: Invalid metric value, must be > 0 " - "in route definition"), - errorDetail); - return NULL; - } - } return virNetDevIPRouteCreate(errorDetail, family, address, netmask, gateway, prefix, hasPrefix, metric, diff --git a/src/conf/networkcommon_conf.h b/src/conf/networkcommon_conf.h index b5955d9f99..9ac981d0d7 100644 --- a/src/conf/networkcommon_conf.h +++ b/src/conf/networkcommon_conf.h @@ -42,8 +42,7 @@ virNetDevIPRouteCreate(const char *networkName, virNetDevIPRoute * virNetDevIPRouteParseXML(const char *networkName, - xmlNodePtr node, - xmlXPathContextPtr ctxt); + xmlNodePtr node); int virNetDevIPRouteFormat(virBuffer *buf, const virNetDevIPRoute *def); -- 2.37.3
On a Monday in 2022, Peter Krempa wrote:
The function extracts multiple attributes form a single element. Modify the function to stop using multiple XPath lookups.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 2 +- src/conf/network_conf.c | 4 +-- src/conf/networkcommon_conf.c | 61 +++++++++-------------------------- src/conf/networkcommon_conf.h | 3 +- 4 files changed, 19 insertions(+), 51 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Parse the 'prefix' field directly and adjust the the error message format strings. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/network_conf.c | 21 +++++---------------- 1 file changed, 5 insertions(+), 16 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 44ac5408d1..b98ae6aa3f 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -1057,8 +1057,6 @@ virNetworkIPDefParseXML(const char *networkName, xmlNodePtr dhcp; g_autofree char *address = NULL; g_autofree char *netmask = NULL; - unsigned long prefix = 0; - int prefixRc; int ret = -1; ctxt->node = node; @@ -1089,17 +1087,8 @@ virNetworkIPDefParseXML(const char *networkName, goto cleanup; } - prefixRc = virXPathULong("string(./@prefix)", ctxt, &prefix); - if (prefixRc == -2) { - virReportError(VIR_ERR_XML_ERROR, - _("Invalid ULong value specified for prefix in definition of network '%s'"), - networkName); + if (virXMLPropUInt(node, "prefix", 10, VIR_XML_PROP_NONE, &def->prefix) < 0) goto cleanup; - } - if (prefixRc < 0) - def->prefix = 0; - else - def->prefix = prefix; if (virXMLPropTristateBool(node, "localPtr", VIR_XML_PROP_NONE, @@ -1131,8 +1120,8 @@ virNetworkIPDefParseXML(const char *networkName, } } else if (def->prefix > 32) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Invalid IPv4 prefix '%lu' in network '%s'"), - prefix, networkName); + _("Invalid IPv4 prefix '%u' in network '%s'"), + def->prefix, networkName); goto cleanup; } } else if (STREQ(def->family, "ipv6")) { @@ -1150,8 +1139,8 @@ virNetworkIPDefParseXML(const char *networkName, } if (def->prefix > 128) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Invalid IPv6 prefix '%lu' in network '%s'"), - prefix, networkName); + _("Invalid IPv6 prefix '%u' in network '%s'"), + def->prefix, networkName); goto cleanup; } } else { -- 2.37.3
On a Monday in 2022, Peter Krempa wrote:
Parse the 'prefix' field directly and adjust the the error message format strings.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/network_conf.c | 21 +++++---------------- 1 file changed, 5 insertions(+), 16 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Use the proper convertor function and refactor error reporting. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/interface_conf.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c index b31fdce101..671b8b088f 100644 --- a/src/conf/interface_conf.c +++ b/src/conf/interface_conf.c @@ -113,17 +113,15 @@ static int virInterfaceDefParseMtu(virInterfaceDef *def, xmlXPathContextPtr ctxt) { - unsigned long mtu; - int ret; + if (virXPathUInt("string(./mtu/@size)", ctxt, &def->mtu) == -2) + return -1; - ret = virXPathULong("string(./mtu/@size)", ctxt, &mtu); - if ((ret == -2) || ((ret == 0) && (mtu > 100000))) { - virReportError(VIR_ERR_XML_ERROR, - "%s", _("interface mtu value is improper")); + if (def->mtu > 100000) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("value of the 'size' attribute of 'mtu' element must be at most 100000")); return -1; - } else if (ret == 0) { - def->mtu = (unsigned int) mtu; } + return 0; } -- 2.37.3
On a Monday in 2022, Peter Krempa wrote:
Use the proper convertor function and refactor error reporting.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/interface_conf.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
The loop inside virNetDevVlanParse fetches multiple attributes from the element. Convert it to use the virXMLProp* helpers, which also simplifies error reporting. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/netdev_vlan_conf.c | 39 ++++++++++++++++--------------------- 1 file changed, 17 insertions(+), 22 deletions(-) diff --git a/src/conf/netdev_vlan_conf.c b/src/conf/netdev_vlan_conf.c index 9d7cc732ba..28a818ad85 100644 --- a/src/conf/netdev_vlan_conf.c +++ b/src/conf/netdev_vlan_conf.c @@ -35,7 +35,6 @@ virNetDevVlanParse(xmlNodePtr node, xmlXPathContextPtr ctxt, virNetDevVlan *def) int ret = -1; VIR_XPATH_NODE_AUTORESTORE(ctxt) char *trunk = NULL; - char *nativeMode = NULL; xmlNodePtr *tagNodes = NULL; int nTags; size_t i; @@ -57,37 +56,34 @@ virNetDevVlanParse(xmlNodePtr node, xmlXPathContextPtr ctxt, virNetDevVlan *def) def->nativeMode = 0; def->nativeTag = 0; for (i = 0; i < nTags; i++) { - unsigned long id; + unsigned int nativeMode = 0; + int rc; - ctxt->node = tagNodes[i]; - if (virXPathULong("string(./@id)", ctxt, &id) < 0) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("missing or invalid vlan tag id attribute")); + if (virXMLPropUInt(tagNodes[i], "id", 10, VIR_XML_PROP_REQUIRED, + &def->tag[i]) < 0) goto cleanup; - } - if (id > 4095) { + + if (def->tag[i] > 4095) { virReportError(VIR_ERR_XML_ERROR, - _("vlan tag id %lu too large (maximum 4095)"), id); + _("vlan tag id %u too large (maximum 4095)"), def->tag[i]); goto cleanup; } - if ((nativeMode = virXPathString("string(./@nativeMode)", ctxt))) { + + if ((rc = virXMLPropEnum(tagNodes[i], "nativeMode", + virNativeVlanModeTypeFromString, + VIR_XML_PROP_NONZERO, &nativeMode)) < 0) + return -1; + + if (rc == 1) { if (def->nativeMode != 0) { virReportError(VIR_ERR_XML_ERROR, "%s", _("duplicate native vlan setting")); goto cleanup; } - if ((def->nativeMode - = virNativeVlanModeTypeFromString(nativeMode)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Invalid \"nativeMode='%s'\" " - "in vlan <tag> element"), - nativeMode); - goto cleanup; - } - VIR_FREE(nativeMode); - def->nativeTag = id; + + def->nativeMode = nativeMode; + def->nativeTag = def->tag[i]; } - def->tag[i] = id; } def->nTags = nTags; @@ -128,7 +124,6 @@ virNetDevVlanParse(xmlNodePtr node, xmlXPathContextPtr ctxt, virNetDevVlan *def) cleanup: VIR_FREE(tagNodes); VIR_FREE(trunk); - VIR_FREE(nativeMode); if (ret < 0) virNetDevVlanClear(def); return ret; -- 2.37.3
On a Monday in 2022, Peter Krempa wrote:
The loop inside virNetDevVlanParse fetches multiple attributes from the element. Convert it to use the virXMLProp* helpers, which also simplifies error reporting.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/netdev_vlan_conf.c | 39 ++++++++++++++++--------------------- 1 file changed, 17 insertions(+), 22 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
The struct used 'unsigned long' variables which we try to avoid due to being different size on different architectures. Convert the stuct and use virXMLPropULongLong instead of virXPathULong when parsing the XML. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 34 +++++++++------------------------- src/conf/domain_conf.h | 6 +++--- 2 files changed, 12 insertions(+), 28 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index cdf05e8fae..ee9eb6bcc4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10610,33 +10610,17 @@ virDomainTimerDefParseXML(xmlNodePtr node, catchup = virXPathNode("./catchup", ctxt); if (catchup != NULL) { - ret = virXPathULong("string(./catchup/@threshold)", ctxt, - &def->catchup.threshold); - if (ret == -1) { - def->catchup.threshold = 0; - } else if (ret < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("invalid catchup threshold")); + if (virXMLPropULongLong(catchup, "threshold", 10, VIR_XML_PROP_NONE, + &def->catchup.threshold) < 0) goto error; - } - ret = virXPathULong("string(./catchup/@slew)", ctxt, &def->catchup.slew); - if (ret == -1) { - def->catchup.slew = 0; - } else if (ret < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("invalid catchup slew")); + if (virXMLPropULongLong(catchup, "slew", 10, VIR_XML_PROP_NONE, + &def->catchup.slew) < 0) goto error; - } - ret = virXPathULong("string(./catchup/@limit)", ctxt, &def->catchup.limit); - if (ret == -1) { - def->catchup.limit = 0; - } else if (ret < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("invalid catchup limit")); + if (virXMLPropULongLong(catchup, "limit", 10, VIR_XML_PROP_NONE, + &def->catchup.limit) < 0) goto error; - } } return def; @@ -24896,11 +24880,11 @@ virDomainTimerDefFormat(virBuffer *buf, } if (def->catchup.threshold > 0) - virBufferAsprintf(&catchupAttr, " threshold='%lu'", def->catchup.threshold); + virBufferAsprintf(&catchupAttr, " threshold='%llu'", def->catchup.threshold); if (def->catchup.slew > 0) - virBufferAsprintf(&catchupAttr, " slew='%lu'", def->catchup.slew); + virBufferAsprintf(&catchupAttr, " slew='%llu'", def->catchup.slew); if (def->catchup.limit > 0) - virBufferAsprintf(&catchupAttr, " limit='%lu'", def->catchup.limit); + virBufferAsprintf(&catchupAttr, " limit='%llu'", def->catchup.limit); virXMLFormatElement(&timerChld, "catchup", &catchupAttr, NULL); virXMLFormatElement(buf, "timer", &timerAttr, &timerChld); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 8f8a54bc41..ba411bfa02 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2466,9 +2466,9 @@ struct _virDomainThreadSchedParam { }; struct _virDomainTimerCatchupDef { - unsigned long threshold; - unsigned long slew; - unsigned long limit; + unsigned long long threshold; + unsigned long long slew; + unsigned long long limit; }; struct _virDomainTimerDef { -- 2.37.3
On a Monday in 2022, Peter Krempa wrote:
The struct used 'unsigned long' variables which we try to avoid due to being different size on different architectures.
Convert the stuct and use virXMLPropULongLong instead of virXPathULong
*struct
when parsing the XML.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 34 +++++++++------------------------- src/conf/domain_conf.h | 6 +++--- 2 files changed, 12 insertions(+), 28 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Automatically free the 'def' variable and remove the 'cleanup' label. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ee9eb6bcc4..7c718ce799 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10542,7 +10542,7 @@ static virDomainTimerDef * virDomainTimerDefParseXML(xmlNodePtr node, xmlXPathContextPtr ctxt) { - virDomainTimerDef *def; + g_autofree virDomainTimerDef *def = g_new0(virDomainTimerDef, 1); VIR_XPATH_NODE_AUTORESTORE(ctxt) xmlNodePtr catchup; int ret; @@ -10551,33 +10551,31 @@ virDomainTimerDefParseXML(xmlNodePtr node, g_autofree char *track = NULL; g_autofree char *mode = NULL; - def = g_new0(virDomainTimerDef, 1); - ctxt->node = node; name = virXMLPropString(node, "name"); if (name == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing timer name")); - goto error; + return NULL; } if ((def->name = virDomainTimerNameTypeFromString(name)) < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unknown timer name '%s'"), name); - goto error; + return NULL; } if (virXMLPropTristateBool(node, "present", VIR_XML_PROP_NONE, &def->present) < 0) - goto error; + return NULL; tickpolicy = virXMLPropString(node, "tickpolicy"); if (tickpolicy != NULL) { if ((def->tickpolicy = virDomainTimerTickpolicyTypeFromString(tickpolicy)) <= 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unknown timer tickpolicy '%s'"), tickpolicy); - goto error; + return NULL; } } @@ -10586,7 +10584,7 @@ virDomainTimerDefParseXML(xmlNodePtr node, if ((def->track = virDomainTimerTrackTypeFromString(track)) <= 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unknown timer track '%s'"), track); - goto error; + return NULL; } } @@ -10596,7 +10594,7 @@ virDomainTimerDefParseXML(xmlNodePtr node, } else if (ret < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("invalid timer frequency")); - goto error; + return NULL; } mode = virXMLPropString(node, "mode"); @@ -10604,7 +10602,7 @@ virDomainTimerDefParseXML(xmlNodePtr node, if ((def->mode = virDomainTimerModeTypeFromString(mode)) <= 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unknown timer mode '%s'"), mode); - goto error; + return NULL; } } @@ -10612,22 +10610,18 @@ virDomainTimerDefParseXML(xmlNodePtr node, if (catchup != NULL) { if (virXMLPropULongLong(catchup, "threshold", 10, VIR_XML_PROP_NONE, &def->catchup.threshold) < 0) - goto error; + return NULL; if (virXMLPropULongLong(catchup, "slew", 10, VIR_XML_PROP_NONE, &def->catchup.slew) < 0) - goto error; + return NULL; if (virXMLPropULongLong(catchup, "limit", 10, VIR_XML_PROP_NONE, &def->catchup.limit) < 0) - goto error; + return NULL; } - return def; - - error: - VIR_FREE(def); - return def; + return g_steal_pointer(&def); } -- 2.37.3
On a Monday in 2022, Peter Krempa wrote:
Automatically free the 'def' variable and remove the 'cleanup' label.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Adjust the type and the corresponding parser to use virXMLPropEnum. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 13 ++----------- src/conf/domain_conf.h | 2 +- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7c718ce799..aa949766e1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10546,24 +10546,15 @@ virDomainTimerDefParseXML(xmlNodePtr node, VIR_XPATH_NODE_AUTORESTORE(ctxt) xmlNodePtr catchup; int ret; - g_autofree char *name = NULL; g_autofree char *tickpolicy = NULL; g_autofree char *track = NULL; g_autofree char *mode = NULL; ctxt->node = node; - name = virXMLPropString(node, "name"); - if (name == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("missing timer name")); + if (virXMLPropEnum(node, "name", virDomainTimerNameTypeFromString, + VIR_XML_PROP_REQUIRED, &def->name) < 0) return NULL; - } - if ((def->name = virDomainTimerNameTypeFromString(name)) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown timer name '%s'"), name); - return NULL; - } if (virXMLPropTristateBool(node, "present", VIR_XML_PROP_NONE, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index ba411bfa02..de6aefed2d 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2472,7 +2472,7 @@ struct _virDomainTimerCatchupDef { }; struct _virDomainTimerDef { - int name; + virDomainTimerNameType name; virTristateBool present; int tickpolicy; /* enum virDomainTimerTickpolicyType */ -- 2.37.3
On a Monday in 2022, Peter Krempa wrote:
Adjust the type and the corresponding parser to use virXMLPropEnum.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 13 ++----------- src/conf/domain_conf.h | 2 +- 2 files changed, 3 insertions(+), 12 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Convert the field, adjust the XML parser to use virXMLPropEnum and add the VIR_DOMAIN_TIMER_TICKPOLICY_LAST enum case to all appropriate 'switch' statements. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 12 +++--------- src/conf/domain_conf.h | 2 +- src/qemu/qemu_command.c | 5 +++++ src/qemu/qemu_validate.c | 6 ++++++ 4 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index aa949766e1..4810aa7b82 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10546,7 +10546,6 @@ virDomainTimerDefParseXML(xmlNodePtr node, VIR_XPATH_NODE_AUTORESTORE(ctxt) xmlNodePtr catchup; int ret; - g_autofree char *tickpolicy = NULL; g_autofree char *track = NULL; g_autofree char *mode = NULL; @@ -10561,14 +10560,9 @@ virDomainTimerDefParseXML(xmlNodePtr node, &def->present) < 0) return NULL; - tickpolicy = virXMLPropString(node, "tickpolicy"); - if (tickpolicy != NULL) { - if ((def->tickpolicy = virDomainTimerTickpolicyTypeFromString(tickpolicy)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown timer tickpolicy '%s'"), tickpolicy); - return NULL; - } - } + if (virXMLPropEnum(node, "tickpolicy", virDomainTimerTickpolicyTypeFromString, + VIR_XML_PROP_NONZERO, &def->tickpolicy) < 0) + return NULL; track = virXMLPropString(node, "track"); if (track != NULL) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index de6aefed2d..2733d12fcd 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2474,7 +2474,7 @@ struct _virDomainTimerCatchupDef { struct _virDomainTimerDef { virDomainTimerNameType name; virTristateBool present; - int tickpolicy; /* enum virDomainTimerTickpolicyType */ + virDomainTimerTickpolicyType tickpolicy; virDomainTimerCatchupDef catchup; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 150824f2e1..7507ab46ab 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5747,6 +5747,8 @@ qemuBuildClockArgStr(virDomainClockDef *def) case VIR_DOMAIN_TIMER_TICKPOLICY_MERGE: case VIR_DOMAIN_TIMER_TICKPOLICY_DISCARD: return NULL; + case VIR_DOMAIN_TIMER_TICKPOLICY_LAST: + break; } break; /* no need to check other timers - there is only one rtc */ } @@ -5818,6 +5820,8 @@ qemuBuildClockCommandLine(virCommand *cmd, case VIR_DOMAIN_TIMER_TICKPOLICY_MERGE: /* no way to support this mode for pit in qemu */ return -1; + case VIR_DOMAIN_TIMER_TICKPOLICY_LAST: + break; } break; @@ -6201,6 +6205,7 @@ qemuBuildCpuCommandLine(virCommand *cmd, case VIR_DOMAIN_TIMER_TICKPOLICY_NONE: case VIR_DOMAIN_TIMER_TICKPOLICY_CATCHUP: case VIR_DOMAIN_TIMER_TICKPOLICY_MERGE: + case VIR_DOMAIN_TIMER_TICKPOLICY_LAST: break; } break; diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 079009ea4b..f8f2a0fd32 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -572,6 +572,8 @@ qemuValidateDomainDefClockTimers(const virDomainDef *def, virDomainTimerTickpolicyTypeToString( timer->tickpolicy)); return -1; + case VIR_DOMAIN_TIMER_TICKPOLICY_LAST: + break; } break; @@ -598,6 +600,8 @@ qemuValidateDomainDefClockTimers(const virDomainDef *def, virDomainTimerTickpolicyTypeToString( timer->tickpolicy)); return -1; + case VIR_DOMAIN_TIMER_TICKPOLICY_LAST: + break; } break; @@ -650,6 +654,8 @@ qemuValidateDomainDefClockTimers(const virDomainDef *def, virDomainTimerNameTypeToString(timer->name), virDomainTimerTickpolicyTypeToString(timer->tickpolicy)); return -1; + case VIR_DOMAIN_TIMER_TICKPOLICY_LAST: + break; } break; } -- 2.37.3
On a Monday in 2022, Peter Krempa wrote:
Convert the field, adjust the XML parser to use virXMLPropEnum and add the VIR_DOMAIN_TIMER_TICKPOLICY_LAST enum case to all appropriate 'switch' statements.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 12 +++--------- src/conf/domain_conf.h | 2 +- src/qemu/qemu_command.c | 5 +++++ src/qemu/qemu_validate.c | 6 ++++++ 4 files changed, 15 insertions(+), 10 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Adjust the parser and add missing switch cases to make the complier happy. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 13 ++++--------- src/conf/domain_conf.h | 2 +- src/qemu/qemu_command.c | 2 ++ src/qemu/qemu_validate.c | 2 ++ 4 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 4810aa7b82..d91c058868 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10546,7 +10546,6 @@ virDomainTimerDefParseXML(xmlNodePtr node, VIR_XPATH_NODE_AUTORESTORE(ctxt) xmlNodePtr catchup; int ret; - g_autofree char *track = NULL; g_autofree char *mode = NULL; ctxt->node = node; @@ -10564,14 +10563,10 @@ virDomainTimerDefParseXML(xmlNodePtr node, VIR_XML_PROP_NONZERO, &def->tickpolicy) < 0) return NULL; - track = virXMLPropString(node, "track"); - if (track != NULL) { - if ((def->track = virDomainTimerTrackTypeFromString(track)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown timer track '%s'"), track); - return NULL; - } - } + if (virXMLPropEnum(node, "track", virDomainTimerTrackTypeFromString, + VIR_XML_PROP_NONZERO, &def->track) < 0) + return NULL; + ret = virXPathULongLong("string(./@frequency)", ctxt, &def->frequency); if (ret == -1) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 2733d12fcd..36b6757c3c 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2479,7 +2479,7 @@ struct _virDomainTimerDef { virDomainTimerCatchupDef catchup; /* track is only valid for name='platform|rtc' */ - int track; /* enum virDomainTimerTrackType */ + virDomainTimerTrackType track; /* frequency & mode are only valid for name='tsc' */ unsigned long long frequency; /* in Hz, unspecified = 0 */ diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 7507ab46ab..bf82878e7a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5732,6 +5732,8 @@ qemuBuildClockArgStr(virDomainClockDef *def) case VIR_DOMAIN_TIMER_TRACK_REALTIME: virBufferAddLit(&buf, ",clock=rt"); break; + case VIR_DOMAIN_TIMER_TRACK_LAST: + break; } switch (def->timers[i]->tickpolicy) { diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index f8f2a0fd32..1456a69351 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -554,6 +554,8 @@ qemuValidateDomainDefClockTimers(const virDomainDef *def, _("unsupported rtc timer track '%s'"), virDomainTimerTrackTypeToString(timer->track)); return -1; + case VIR_DOMAIN_TIMER_TRACK_LAST: + break; } switch (timer->tickpolicy) { -- 2.37.3
On a Monday in 2022, Peter Krempa wrote:
Adjust the parser and add missing switch cases to make the complier happy.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 13 ++++--------- src/conf/domain_conf.h | 2 +- src/qemu/qemu_command.c | 2 ++ src/qemu/qemu_validate.c | 2 ++ 4 files changed, 9 insertions(+), 10 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Adjust the parser and switch statements to go with it. Note that the XEN/libxl drivers had a 'default:' case for few of the swtich statements so this patch blindly expands it to what it would be in those cases. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 12 +++--------- src/conf/domain_conf.h | 2 +- src/libxl/libxl_conf.c | 6 +++++- src/libxl/xen_common.c | 6 +++++- 4 files changed, 14 insertions(+), 12 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d91c058868..6c976b366a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10546,7 +10546,6 @@ virDomainTimerDefParseXML(xmlNodePtr node, VIR_XPATH_NODE_AUTORESTORE(ctxt) xmlNodePtr catchup; int ret; - g_autofree char *mode = NULL; ctxt->node = node; @@ -10577,14 +10576,9 @@ virDomainTimerDefParseXML(xmlNodePtr node, return NULL; } - mode = virXMLPropString(node, "mode"); - if (mode != NULL) { - if ((def->mode = virDomainTimerModeTypeFromString(mode)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown timer mode '%s'"), mode); - return NULL; - } - } + if (virXMLPropEnum(node, "mode", virDomainTimerModeTypeFromString, + VIR_XML_PROP_NONZERO, &def->mode) < 0) + return NULL; catchup = virXPathNode("./catchup", ctxt); if (catchup != NULL) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 36b6757c3c..f5825138e7 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2483,7 +2483,7 @@ struct _virDomainTimerDef { /* frequency & mode are only valid for name='tsc' */ unsigned long long frequency; /* in Hz, unspecified = 0 */ - int mode; /* enum virDomainTimerModeType */ + virDomainTimerModeType mode; }; typedef enum { diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 918303c8d0..d13e48abb2 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -409,8 +409,12 @@ libxlMakeDomBuildInfo(virDomainDef *def, case VIR_DOMAIN_TIMER_MODE_EMULATE: b_info->tsc_mode = LIBXL_TSC_MODE_ALWAYS_EMULATE; break; - default: + case VIR_DOMAIN_TIMER_MODE_NONE: + case VIR_DOMAIN_TIMER_MODE_AUTO: + case VIR_DOMAIN_TIMER_MODE_SMPSAFE: b_info->tsc_mode = LIBXL_TSC_MODE_DEFAULT; + case VIR_DOMAIN_TIMER_MODE_LAST: + break; } break; diff --git a/src/libxl/xen_common.c b/src/libxl/xen_common.c index 6918a4b121..f61719adac 100644 --- a/src/libxl/xen_common.c +++ b/src/libxl/xen_common.c @@ -2102,9 +2102,13 @@ xenFormatHypervisorFeatures(virConf *conf, virDomainDef *def) if (xenConfigSetString(conf, "tsc_mode", "always_emulate") < 0) return -1; break; - default: + case VIR_DOMAIN_TIMER_MODE_NONE: + case VIR_DOMAIN_TIMER_MODE_AUTO: + case VIR_DOMAIN_TIMER_MODE_SMPSAFE: if (xenConfigSetString(conf, "tsc_mode", "default") < 0) return -1; + case VIR_DOMAIN_TIMER_MODE_LAST: + break; } break; -- 2.37.3
On a Monday in 2022, Peter Krempa wrote:
Adjust the parser and switch statements to go with it.
Note that the XEN/libxl drivers had a 'default:' case for few of the swtich statements so this patch blindly expands it to what it would be in those cases.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 12 +++--------- src/conf/domain_conf.h | 2 +- src/libxl/libxl_conf.c | 6 +++++- src/libxl/xen_common.c | 6 +++++- 4 files changed, 14 insertions(+), 12 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Parse the 'frequency' field without an extra XPath. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6c976b366a..2d01693a31 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10545,7 +10545,6 @@ virDomainTimerDefParseXML(xmlNodePtr node, g_autofree virDomainTimerDef *def = g_new0(virDomainTimerDef, 1); VIR_XPATH_NODE_AUTORESTORE(ctxt) xmlNodePtr catchup; - int ret; ctxt->node = node; @@ -10566,15 +10565,8 @@ virDomainTimerDefParseXML(xmlNodePtr node, VIR_XML_PROP_NONZERO, &def->track) < 0) return NULL; - - ret = virXPathULongLong("string(./@frequency)", ctxt, &def->frequency); - if (ret == -1) { - def->frequency = 0; - } else if (ret < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("invalid timer frequency")); + if (virXMLPropULongLong(node, "frequency", 10, VIR_XML_PROP_NONE, &def->frequency) < 0) return NULL; - } if (virXMLPropEnum(node, "mode", virDomainTimerModeTypeFromString, VIR_XML_PROP_NONZERO, &def->mode) < 0) -- 2.37.3
On a Monday in 2022, Peter Krempa wrote:
Parse the 'frequency' field without an extra XPath.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Use 'unsigned long long' instead of 'unsigned long' and fix the parser and formatter. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 4 ++-- src/conf/domain_conf.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2d01693a31..7c6caa01e5 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9376,7 +9376,7 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, if (virDomainNetTeamingInfoParseXML(ctxt, &def->teaming) < 0) return NULL; - rv = virXPathULong("string(./tune/sndbuf)", ctxt, &def->tune.sndbuf); + rv = virXPathULongLong("string(./tune/sndbuf)", ctxt, &def->tune.sndbuf); if (rv >= 0) { def->tune.sndbuf_specified = true; } else if (rv == -2) { @@ -23585,7 +23585,7 @@ virDomainNetDefFormat(virBuffer *buf, if (def->tune.sndbuf_specified) { virBufferAddLit(buf, "<tune>\n"); virBufferAdjustIndent(buf, 2); - virBufferAsprintf(buf, "<sndbuf>%lu</sndbuf>\n", def->tune.sndbuf); + virBufferAsprintf(buf, "<sndbuf>%llu</sndbuf>\n", def->tune.sndbuf); virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</tune>\n"); } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index f5825138e7..54dc9098df 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1146,7 +1146,7 @@ struct _virDomainNetDef { virNetDevVPortProfile *virtPortProfile; struct { bool sndbuf_specified; - unsigned long sndbuf; + unsigned long long sndbuf; } tune; char *script; char *downscript; -- 2.37.3
On a Monday in 2022, Peter Krempa wrote:
Use 'unsigned long long' instead of 'unsigned long' and fix the parser and formatter.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 4 ++-- src/conf/domain_conf.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Use the proper function for an unsigned int. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7c6caa01e5..8b1a28c0d3 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13178,14 +13178,13 @@ static int virDomainSEVDefParseXML(virDomainSEVDef *def, xmlXPathContextPtr ctxt) { - unsigned long policy; int rc; if (virXMLPropTristateBool(ctxt->node, "kernelHashes", VIR_XML_PROP_NONE, &def->kernel_hashes) < 0) return -1; - if (virXPathULongHex("string(./policy)", ctxt, &policy) < 0) { + if (virXPathUIntBase("string(./policy)", ctxt, 16, &def->policy) < 0) { virReportError(VIR_ERR_XML_ERROR, "%s", _("failed to get launch security policy")); return -1; @@ -13214,7 +13213,6 @@ virDomainSEVDefParseXML(virDomainSEVDef *def, return -1; } - def->policy = policy; def->dh_cert = virXPathString("string(./dhCert)", ctxt); def->session = virXPathString("string(./session)", ctxt); -- 2.37.3
On a Monday in 2022, Peter Krempa wrote:
Use the proper function for an unsigned int.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
We don't need to do the extra XPath lookups and we can use the proper type right away. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/cpu/cpu_ppc64.c | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c index 7da67ec94a..30db99cca1 100644 --- a/src/cpu/cpu_ppc64.c +++ b/src/cpu/cpu_ppc64.c @@ -305,7 +305,6 @@ ppc64ModelParse(xmlXPathContextPtr ctxt, g_autoptr(virCPUppc64Model) model = NULL; g_autofree xmlNodePtr *nodes = NULL; g_autofree char *vendor = NULL; - unsigned long pvr; size_t i; int n; @@ -346,23 +345,13 @@ ppc64ModelParse(xmlXPathContextPtr ctxt, model->data.len = n; for (i = 0; i < n; i++) { - ctxt->node = nodes[i]; - - if (virXPathULongHex("string(./@value)", ctxt, &pvr) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Missing or invalid PVR value in CPU model %s"), - model->name); + if (virXMLPropUInt(nodes[i], "value", 16, VIR_XML_PROP_REQUIRED, + &model->data.pvr[i].value) < 0) return -1; - } - model->data.pvr[i].value = pvr; - if (virXPathULongHex("string(./@mask)", ctxt, &pvr) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Missing or invalid PVR mask in CPU model %s"), - model->name); + if (virXMLPropUInt(nodes[i], "mask", 16, VIR_XML_PROP_REQUIRED, + &model->data.pvr[i].mask) < 0) return -1; - } - model->data.pvr[i].mask = pvr; } VIR_APPEND_ELEMENT(map->models, map->nmodels, model); -- 2.37.3
On a Monday in 2022, Peter Krempa wrote:
We don't need to do the extra XPath lookups and we can use the proper type right away.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/cpu/cpu_ppc64.c | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Use the function for the proper type. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 9ef6c8bb64..c3afc6c9d3 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2764,7 +2764,6 @@ qemuDomainObjPrivateXMLParseBlockjobData(virDomainObj *vm, int newstate = -1; bool invalidData = false; xmlNodePtr tmp; - unsigned long jobflags = 0; ctxt->node = node; @@ -2804,7 +2803,8 @@ qemuDomainObjPrivateXMLParseBlockjobData(virDomainObj *vm, STRNEQ(mirror, "yes")) invalidData = true; - if (virXPathULongHex("string(./@jobflags)", ctxt, &jobflags) != 0) + if (virXMLPropUInt(ctxt->node, "jobflags", 16, VIR_XML_PROP_NONE, + &job->jobflags) != 1) job->jobflagsmissing = true; if (!disk && !invalidData) { @@ -2826,7 +2826,6 @@ qemuDomainObjPrivateXMLParseBlockjobData(virDomainObj *vm, job->state = state; job->newstate = newstate; - job->jobflags = jobflags; job->errmsg = virXPathString("string(./errmsg)", ctxt); job->invalidData = invalidData; job->disk = disk; -- 2.37.3
On a Monday in 2022, Peter Krempa wrote:
Use the function for the proper type.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
The callers store only an 'unsigned int' in the field. Convert it to the proper type including parser/formatter. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/virdomainjob.c | 4 ++-- src/conf/virdomainjob.h | 4 ++-- src/qemu/qemu_domainjob.c | 7 +++---- src/qemu/qemu_migration.c | 2 +- src/qemu/qemu_migration_params.c | 8 ++++---- src/qemu/qemu_migration_params.h | 4 ++-- src/qemu/qemu_process.c | 2 +- src/qemu/qemu_process.h | 2 +- 8 files changed, 16 insertions(+), 17 deletions(-) diff --git a/src/conf/virdomainjob.c b/src/conf/virdomainjob.c index ca0215bc23..256b665a42 100644 --- a/src/conf/virdomainjob.c +++ b/src/conf/virdomainjob.c @@ -431,7 +431,7 @@ virDomainObjBeginJobInternal(virDomainObj *obj, VIR_WARN("Cannot start job (%s, %s, %s) in API %s for domain %s; " "current job is (%s, %s, %s) " - "owned by (%llu %s, %llu %s, %llu %s (flags=0x%lx)) " + "owned by (%llu %s, %llu %s, %llu %s (flags=0x%x)) " "for (%llus, %llus, %llus)", virDomainJobTypeToString(job), virDomainAgentJobTypeToString(agentJob), @@ -550,7 +550,7 @@ virDomainObjBeginAgentJob(virDomainObj *obj, int virDomainObjBeginAsyncJob(virDomainObj *obj, virDomainAsyncJob asyncJob, virDomainJobOperation operation, - unsigned long apiFlags) + unsigned int apiFlags) { if (virDomainObjBeginJobInternal(obj, obj->job, VIR_JOB_ASYNC, VIR_AGENT_JOB_NONE, diff --git a/src/conf/virdomainjob.h b/src/conf/virdomainjob.h index c101334596..b1ac36a2fa 100644 --- a/src/conf/virdomainjob.h +++ b/src/conf/virdomainjob.h @@ -182,7 +182,7 @@ struct _virDomainJobObj { virDomainJobData *completed; /* statistics data of a recently completed job */ bool abortJob; /* abort of the job requested */ char *error; /* job event completion error */ - unsigned long apiFlags; /* flags passed to the API which started the async job */ + unsigned int apiFlags; /* flags passed to the API which started the async job */ void *privateData; /* job specific collection of data */ virDomainObjPrivateJobCallbacks *cb; @@ -255,7 +255,7 @@ int virDomainObjBeginAgentJob(virDomainObj *obj, int virDomainObjBeginAsyncJob(virDomainObj *obj, virDomainAsyncJob asyncJob, virDomainJobOperation operation, - unsigned long apiFlags) + unsigned int apiFlags) G_GNUC_WARN_UNUSED_RESULT; int virDomainObjBeginNestedJob(virDomainObj *obj, virDomainAsyncJob asyncJob) diff --git a/src/qemu/qemu_domainjob.c b/src/qemu/qemu_domainjob.c index a170fdd97d..4d320f075e 100644 --- a/src/qemu/qemu_domainjob.c +++ b/src/qemu/qemu_domainjob.c @@ -692,7 +692,7 @@ qemuDomainObjPrivateXMLFormatJob(virBuffer *buf, } if (vm->job->asyncJob != VIR_ASYNC_JOB_NONE) { - virBufferAsprintf(&attrBuf, " flags='0x%lx'", vm->job->apiFlags); + virBufferAsprintf(&attrBuf, " flags='0x%x'", vm->job->apiFlags); virBufferAsprintf(&attrBuf, " asyncStarted='%llu'", vm->job->asyncStarted); } @@ -758,10 +758,9 @@ qemuDomainObjPrivateXMLParseJob(virDomainObj *vm, } } - if (virXPathULongHex("string(@flags)", ctxt, &vm->job->apiFlags) == -2) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid job flags")); + if (virXMLPropUInt(ctxt->node, "flags", 16, VIR_XML_PROP_NONE, + &vm->job->apiFlags) < 0) return -1; - } if (vm->job->cb && vm->job->cb->parseJobPrivate(ctxt, job, vm) < 0) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 33105cf07b..53c655fb93 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -101,7 +101,7 @@ qemuMigrationJobIsAllowed(virDomainObj *vm) static int ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) G_GNUC_WARN_UNUSED_RESULT qemuMigrationJobStart(virDomainObj *vm, virDomainAsyncJob job, - unsigned long apiFlags) + unsigned int apiFlags) { virDomainJobOperation op; unsigned long long mask; diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c index 7a023b36c8..bee0af9fca 100644 --- a/src/qemu/qemu_migration_params.c +++ b/src/qemu/qemu_migration_params.c @@ -919,7 +919,7 @@ int qemuMigrationParamsApply(virDomainObj *vm, int asyncJob, qemuMigrationParams *migParams, - unsigned long apiFlags) + unsigned int apiFlags) { bool postcopyResume = !!(apiFlags & VIR_MIGRATE_POSTCOPY_RESUME); int ret = -1; @@ -1117,7 +1117,7 @@ static void qemuMigrationParamsResetTLS(virDomainObj *vm, int asyncJob, qemuMigrationParams *origParams, - unsigned long apiFlags) + unsigned int apiFlags) { g_autofree char *tlsAlias = NULL; g_autofree char *secAlias = NULL; @@ -1280,7 +1280,7 @@ void qemuMigrationParamsReset(virDomainObj *vm, int asyncJob, qemuMigrationParams *origParams, - unsigned long apiFlags) + unsigned int apiFlags) { virErrorPtr err; g_autoptr(virBitmap) clearCaps = NULL; @@ -1288,7 +1288,7 @@ qemuMigrationParamsReset(virDomainObj *vm, virErrorPreserveLast(&err); - VIR_DEBUG("Resetting migration parameters %p, flags 0x%lx", + VIR_DEBUG("Resetting migration parameters %p, flags 0x%x", origParams, apiFlags); if (!virDomainObjIsActive(vm) || !origParams) diff --git a/src/qemu/qemu_migration_params.h b/src/qemu/qemu_migration_params.h index c58a933ab6..5612a4d283 100644 --- a/src/qemu/qemu_migration_params.h +++ b/src/qemu/qemu_migration_params.h @@ -98,7 +98,7 @@ int qemuMigrationParamsApply(virDomainObj *vm, int asyncJob, qemuMigrationParams *migParams, - unsigned long apiFlags); + unsigned int apiFlags); int qemuMigrationParamsEnableTLS(virQEMUDriver *driver, @@ -145,7 +145,7 @@ void qemuMigrationParamsReset(virDomainObj *vm, int asyncJob, qemuMigrationParams *origParams, - unsigned long apiFlags); + unsigned int apiFlags); void qemuMigrationParamsFormat(virBuffer *buf, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index f405326312..48b3e07f09 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4689,7 +4689,7 @@ qemuProcessIncomingDefNew(virQEMUCaps *qemuCaps, int qemuProcessBeginJob(virDomainObj *vm, virDomainJobOperation operation, - unsigned long apiFlags) + unsigned int apiFlags) { if (virDomainObjBeginAsyncJob(vm, VIR_ASYNC_JOB_START, operation, apiFlags) < 0) diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index 4dfb2485c0..d1f58ee258 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -65,7 +65,7 @@ void qemuProcessIncomingDefFree(qemuProcessIncomingDef *inc); int qemuProcessBeginJob(virDomainObj *vm, virDomainJobOperation operation, - unsigned long apiFlags); + unsigned int apiFlags); void qemuProcessEndJob(virDomainObj *vm); typedef enum { -- 2.37.3
On a Monday in 2022, Peter Krempa wrote:
The callers store only an 'unsigned int' in the field. Convert it to the proper type including parser/formatter.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/virdomainjob.c | 4 ++-- src/conf/virdomainjob.h | 4 ++-- src/qemu/qemu_domainjob.c | 7 +++---- src/qemu/qemu_migration.c | 2 +- src/qemu/qemu_migration_params.c | 8 ++++---- src/qemu/qemu_migration_params.h | 4 ++-- src/qemu/qemu_process.c | 2 +- src/qemu/qemu_process.h | 2 +- 8 files changed, 16 insertions(+), 17 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Covert all use of 'unsigned long' to 'unsigned long long'. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/cpu/cpu_arm.c | 37 +++++++++++++++---------------------- src/cpu/cpu_arm_data.h | 4 ++-- 2 files changed, 17 insertions(+), 24 deletions(-) diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c index 409b397155..ef72d6dceb 100644 --- a/src/cpu/cpu_arm.c +++ b/src/cpu/cpu_arm.c @@ -57,7 +57,7 @@ static const virArch archs[] = { typedef struct _virCPUarmVendor virCPUarmVendor; struct _virCPUarmVendor { char *name; - unsigned long value; + unsigned long long value; }; typedef struct _virCPUarmModel virCPUarmModel; @@ -266,7 +266,7 @@ virCPUarmMapFeatureParse(xmlXPathContextPtr ctxt G_GNUC_UNUSED, static virCPUarmVendor * virCPUarmVendorFindByID(virCPUarmMap *map, - unsigned long vendor_id) + unsigned long long vendor_id) { size_t i; @@ -312,15 +312,13 @@ virCPUarmVendorParse(xmlXPathContextPtr ctxt, return -1; } - if (virXPathULongHex("string(@value)", ctxt, &vendor->value) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("Missing CPU vendor value")); + if (virXMLPropULongLong(ctxt->node, "value", 16, VIR_XML_PROP_REQUIRED, + &vendor->value) < 0) return -1; - } if (virCPUarmVendorFindByID(map, vendor->value)) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("CPU vendor value 0x%2lx already defined"), + _("CPU vendor value 0x%2llx already defined"), vendor->value); return -1; } @@ -347,7 +345,7 @@ virCPUarmModelFind(virCPUarmMap *map, #if defined(__aarch64__) static virCPUarmModel * virCPUarmModelFindByPVR(virCPUarmMap *map, - unsigned long pvr) + unsigned long long pvr) { size_t i; @@ -367,7 +365,8 @@ virCPUarmModelParse(xmlXPathContextPtr ctxt, { virCPUarmMap *map = data; g_autoptr(virCPUarmModel) model = NULL; - g_autofree char *vendor = NULL; + xmlNodePtr vendorNode = NULL; + xmlNodePtr pvrNode = NULL; model = g_new0(virCPUarmModel, 1); model->name = g_strdup(name); @@ -379,14 +378,11 @@ virCPUarmModelParse(xmlXPathContextPtr ctxt, return -1; } - if (virXPathBoolean("boolean(./vendor)", ctxt)) { - vendor = virXPathString("string(./vendor/@name)", ctxt); - if (!vendor) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Invalid vendor element in CPU model %s"), - model->name); + if ((vendorNode = virXPathNode("./vendor", ctxt))) { + g_autofree char *vendor = NULL; + + if (!(vendor = virXMLPropStringRequired(vendorNode, "name"))) return -1; - } if (!(model->vendor = virCPUarmVendorFindByName(map, vendor))) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -396,19 +392,16 @@ virCPUarmModelParse(xmlXPathContextPtr ctxt, } } - if (!virXPathBoolean("boolean(./pvr)", ctxt)) { + if (!(pvrNode = virXPathNode("./pvr", ctxt))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Missing PVR information for CPU model %s"), model->name); return -1; } - if (virXPathULongHex("string(./pvr/@value)", ctxt, &model->data.pvr) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Missing or invalid PVR value in CPU model %s"), - model->name); + if (virXMLPropULongLong(pvrNode, "value", 16, VIR_XML_PROP_REQUIRED, + &model->data.pvr) < 0) return -1; - } VIR_APPEND_ELEMENT(map->models, map->nmodels, model); diff --git a/src/cpu/cpu_arm_data.h b/src/cpu/cpu_arm_data.h index 36f1a60533..15a2c72d1c 100644 --- a/src/cpu/cpu_arm_data.h +++ b/src/cpu/cpu_arm_data.h @@ -25,7 +25,7 @@ typedef struct _virCPUarmData virCPUarmData; struct _virCPUarmData { - unsigned long vendor_id; - unsigned long pvr; + unsigned long long vendor_id; + unsigned long long pvr; char **features; }; -- 2.37.3
On a Monday in 2022, Peter Krempa wrote:
Covert all use of 'unsigned long' to 'unsigned long long'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/cpu/cpu_arm.c | 37 +++++++++++++++---------------------- src/cpu/cpu_arm_data.h | 4 ++-- 2 files changed, 17 insertions(+), 24 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Remove the now-unused functions for parsing 'unsigned long' values via XPath. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt_private.syms | 2 -- src/util/virxml.c | 75 ---------------------------------------- src/util/virxml.h | 8 ----- 3 files changed, 85 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 0fc5481a26..5bc58d9281 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3699,8 +3699,6 @@ virXPathNodeSet; virXPathString; virXPathUInt; virXPathUIntBase; -virXPathULong; -virXPathULongHex; virXPathULongLong; virXPathULongLongBase; diff --git a/src/util/virxml.c b/src/util/virxml.c index f5ee9284a6..ca83ea1dd5 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -142,38 +142,6 @@ virXPathInt(const char *xpath, } -static int -virXPathULongBase(const char *xpath, - xmlXPathContextPtr ctxt, - int base, - unsigned long *value) -{ - g_autoptr(xmlXPathObject) obj = NULL; - int ret = 0; - - if ((ctxt == NULL) || (xpath == NULL) || (value == NULL)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("Invalid parameter to virXPathULong()")); - return -1; - } - obj = xmlXPathEval(BAD_CAST xpath, ctxt); - if ((obj != NULL) && (obj->type == XPATH_STRING) && - (obj->stringval != NULL) && (obj->stringval[0] != 0)) { - if (virStrToLong_ul((char *) obj->stringval, NULL, base, value) < 0) - ret = -2; - } else if ((obj != NULL) && (obj->type == XPATH_NUMBER) && - (!(isnan(obj->floatval)))) { - *value = (unsigned long) obj->floatval; - if (*value != obj->floatval) - ret = -2; - } else { - ret = -1; - } - - return ret; -} - - /** * virXPathUIntBase: * @xpath: the XPath string to evaluate @@ -216,49 +184,6 @@ virXPathUInt(const char *xpath, } -/** - * virXPathULong: - * @xpath: the XPath string to evaluate - * @ctxt: an XPath context - * @value: the returned long value - * - * Convenience function to evaluate an XPath number - * - * Returns 0 in case of success in which case @value is set, - * or -1 if the XPath evaluation failed or -2 if the - * value doesn't have a long format. - */ -int -virXPathULong(const char *xpath, - xmlXPathContextPtr ctxt, - unsigned long *value) -{ - return virXPathULongBase(xpath, ctxt, 10, value); -} - - -/** - * virXPathUHex: - * @xpath: the XPath string to evaluate - * @ctxt: an XPath context - * @value: the returned long value - * - * Convenience function to evaluate an XPath number - * according to base of 16 - * - * Returns 0 in case of success in which case @value is set, - * or -1 if the XPath evaluation failed or -2 if the - * value doesn't have a long format. - */ -int -virXPathULongHex(const char *xpath, - xmlXPathContextPtr ctxt, - unsigned long *value) -{ - return virXPathULongBase(xpath, ctxt, 16, value); -} - - /** * virXPathULongLongBase: * @xpath: the XPath string to evaluate diff --git a/src/util/virxml.h b/src/util/virxml.h index 439a2a9991..f19cbe59ae 100644 --- a/src/util/virxml.h +++ b/src/util/virxml.h @@ -60,10 +60,6 @@ virXPathUInt(const char *xpath, xmlXPathContextPtr ctxt, unsigned int *value); int -virXPathULong(const char *xpath, - xmlXPathContextPtr ctxt, - unsigned long *value); -int virXPathULongLongBase(const char *xpath, xmlXPathContextPtr ctxt, unsigned int base, @@ -76,10 +72,6 @@ int virXPathLongLong(const char *xpath, xmlXPathContextPtr ctxt, long long *value); -int -virXPathULongHex(const char *xpath, - xmlXPathContextPtr ctxt, - unsigned long *value); xmlNodePtr virXPathNode(const char *xpath, xmlXPathContextPtr ctxt); -- 2.37.3
On a Monday in 2022, Peter Krempa wrote:
Remove the now-unused functions for parsing 'unsigned long' values via XPath.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt_private.syms | 2 -- src/util/virxml.c | 75 ---------------------------------------- src/util/virxml.h | 8 ----- 3 files changed, 85 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Peter Krempa