Most of our completers used the pattern: if ((nITEM = virITEMListAll()) < 0) return NULL; but the virDomainSnapshot and virStorageVolume completers were instead using goto error because of a prior allocation. If the ListAll fails with -1, the cleanup label was running a loop of 'size_t i < int nITEM', which is an extreme waste of CPU cycles. Reported-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Eric Blake <eblake@redhat.com> --- This one is a bug fix, so worth having in 5.2. tools/virsh-completer.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c index c4adbb70d0..e9ef9b99f9 100644 --- a/tools/virsh-completer.c +++ b/tools/virsh-completer.c @@ -278,6 +278,7 @@ virshStorageVolNameCompleter(vshControl *ctl, virshControlPtr priv = ctl->privData; virStoragePoolPtr pool = NULL; virStorageVolPtr *vols = NULL; + int rc; int nvols = 0; size_t i = 0; char **ret = NULL; @@ -290,8 +291,9 @@ virshStorageVolNameCompleter(vshControl *ctl, if (!(pool = virshCommandOptPool(ctl, cmd, "pool", NULL))) return NULL; - if ((nvols = virStoragePoolListAllVolumes(pool, &vols, flags)) < 0) + if ((rc = virStoragePoolListAllVolumes(pool, &vols, flags)) < 0) goto error; + nvols = rc; if (VIR_ALLOC_N(ret, nvols + 1) < 0) goto error; @@ -631,6 +633,7 @@ virshSnapshotNameCompleter(vshControl *ctl, virshControlPtr priv = ctl->privData; virDomainPtr dom = NULL; virDomainSnapshotPtr *snapshots = NULL; + int rc; int nsnapshots = 0; size_t i = 0; char **ret = NULL; @@ -643,8 +646,9 @@ virshSnapshotNameCompleter(vshControl *ctl, if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) return NULL; - if ((nsnapshots = virDomainListAllSnapshots(dom, &snapshots, flags)) < 0) + if ((rc = virDomainListAllSnapshots(dom, &snapshots, flags)) < 0) goto error; + nsnapshots = rc; if (VIR_ALLOC_N(ret, nsnapshots + 1) < 0) goto error; -- 2.20.1