On a Wednesday in 2020, Michal Privoznik wrote:
This is convenience macro, use it more. This commit was generated using the following spatch:@@ symbol node; identifier old; identifier ctxt; type xmlNodePtr; @@ - xmlNodePtr old; + VIR_XPATH_NODE_AUTORESTORE(ctxt); ... - old = ctxt->node; ... when != old - ctxt->node = old; @@ symbol node; identifier old; identifier ctxt; type xmlNodePtr; @@ - xmlNodePtr old = ctxt->node; + VIR_XPATH_NODE_AUTORESTORE(ctxt); ... when != old - ctxt->node = old; Signed-off-by: Michal Privoznik <[email protected]> --- src/conf/cpu_conf.c | 3 +- src/conf/domain_conf.c | 7 +-- src/conf/interface_conf.c | 16 ++----- src/conf/netdev_vlan_conf.c | 3 +- src/conf/network_conf.c | 25 +++------- src/conf/networkcommon_conf.c | 4 +- src/conf/node_device_conf.c | 84 +++++++++------------------------ src/conf/numa_conf.c | 3 +- src/conf/snapshot_conf.c | 3 +- src/conf/storage_adapter_conf.c | 3 +- src/conf/storage_conf.c | 4 +- src/conf/virsavecookie.c | 3 +- src/cpu/cpu_map.c | 12 +---- src/cpu/cpu_x86.c | 3 +- src/lxc/lxc_domain.c | 4 +- src/util/virstorageencryption.c | 8 +--- src/util/virstoragefile.c | 3 +- tools/virsh-domain.c | 6 +-- 18 files changed, 52 insertions(+), 142 deletions(-)
@@ -797,12 +793,12 @@ virNodeDevCapStorageParseXML(xmlXPathContextPtr ctxt,
xmlNodePtr node,
virNodeDevCapStoragePtr storage)
{
- xmlNodePtr orignode, *nodes = NULL;
+ VIR_XPATH_NODE_AUTORESTORE(ctxt);
+ xmlNodePtr *nodes = NULL;
size_t i;
int n, ret = -1;
unsigned long long val;
- orignode = ctxt->node;
ctxt->node = node;
storage->block = virXPathString("string(./block[1])", ctxt);
@@ -835,11 +831,8 @@ virNodeDevCapStorageParseXML(xmlXPathContextPtr ctxt,
if (STREQ(type, "hotpluggable")) {
storage->flags |= VIR_NODE_DEV_CAP_STORAGE_HOTPLUGGABLE;
} else if (STREQ(type, "removable")) {
- xmlNodePtr orignode2;
-
storage->flags |= VIR_NODE_DEV_CAP_STORAGE_REMOVABLE;
- orignode2 = ctxt->node;
This set orignode2 to 'node'
ctxt->node = nodes[i];
if (virXPathBoolean("count(./media_available[. = '1']) > 0", ctxt))
@@ -851,13 +844,10 @@ virNodeDevCapStorageParseXML(xmlXPathContextPtr ctxt,
if (virNodeDevCapsDefParseULongLong("number(./media_size[1])", ctxt,
&val, def,
_("no removable media size supplied
for '%s'"),
_("invalid removable media size
supplied for '%s'")) < 0) {
- ctxt->node = orignode2;
VIR_FREE(type);
goto out;
}
storage->removable_media_size = val;
-
- ctxt->node = orignode2;
This restored it back to 'node'. The new code can possibly leave it at node[i], but the code below using the context is specifically run only for non-removable devices. My sympathy to anyone touching this code in the future :)
} else {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("unknown storage capability type '%s' for '%s'"),
@@ -881,7 +871,6 @@ virNodeDevCapStorageParseXML(xmlXPathContextPtr ctxt,
ret = 0;
out:
VIR_FREE(nodes);
- ctxt->node = orignode;
return ret;
}
[...]
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 61cd72f714..069e3de064 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -6818,7 +6818,7 @@ virshDomainGetVcpuBitmap(vshControl *ctl,
xmlDocPtr xml = NULL;
xmlXPathContextPtr ctxt = NULL;
xmlNodePtr *nodes = NULL;
- xmlNodePtr old;
+ VIR_XPATH_NODE_AUTORESTORE(ctxt);
==1644488== Invalid read of size 8 ==1644488== at 0x169BE9: virshDomainGetVcpuBitmap (virsh-domain.c:6821) ==1644488== by 0x16965A: virshVcpuinfoInactive (virsh-domain.c:6898) ==1644488== by 0x160132: cmdVcpuinfo (virsh-domain.c:6970) ==1644488== by 0x19890E: vshCommandRun (vsh.c:1291) ==1644488== by 0x1362F9: main (virsh.c:905) ==1644488== Address 0x8 is not stack'd, malloc'd or (recently) free'd Since 'ctxt' is freed at the end of the function, there is not much point in restoring it.
int nnodes;
size_t i;
unsigned int curvcpus = 0;
@@ -6853,8 +6853,6 @@ virshDomainGetVcpuBitmap(vshControl *ctl,
goto cleanup;
}
- old = ctxt->node;
-
for (i = 0; i < nnodes; i++) {
ctxt->node = nodes[i];
@@ -6868,8 +6866,6 @@ virshDomainGetVcpuBitmap(vshControl *ctl,
VIR_FREE(online);
}
- ctxt->node = old;
-
if (virBitmapCountBits(ret) != curvcpus) {
vshError(ctl, "%s", _("Failed to retrieve vcpu state bitmap"));
virBitmapFree(ret);
With the crash fixed: Reviewed-by: Ján Tomko <[email protected]> Jano
signature.asc
Description: PGP signature
