3:33 p.m.
Ján Tomko (14):
virsh: fix indentation of info_managed_save_edit
virsh-completer: fix typo
virsh-completer: reword documentation
virsh-completer: switch to using tmp instead of ret
virsh-completer: add a cleanup label everywhere
virsh-completer: unify cleanup of items in name completers
virsh-completer: virStringListFree where possible
virsh-completer: use VIR_AUTOSTRINGLIST for tmp
use VIR_AUTOFREE for xmlNodePtr* variables
virsh-completer: use VIR_AUTOPTR for xml* variables
virsh-completer: use VIR_AUTOFREE for char* variables
virsh-completer: remove excessive labels
virsh-completer: introduce virshPagesizeNodeToString
virsh-completer: introduce virshPasswordCompleter
tools/virsh-completer.c | 611 +++++++++++++++++++---------------------
tools/virsh-completer.h | 4 +
tools/virsh-domain.c | 15 +-
3 files changed, 295 insertions(+), 335 deletions(-)
--
2.20.1
3:33 p.m.
New subject: [libvirt] [PATCH 01/14] virsh: fix indentation of info_managed_save_edit
Use four spaces instead of three.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
tools/virsh-domain.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index afcd0a5f8d..e8d5404acf 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -4805,13 +4805,13 @@ cmdManagedSaveRemove(vshControl *ctl, const vshCmd *cmd)
* "managedsave-edit" command
*/
static const vshCmdInfo info_managed_save_edit[] = {
- {.name = "help",
- .data = N_("edit XML for a domain's managed save state file")
- },
- {.name = "desc",
- .data = N_("Edit the domain XML associated with the managed save state
file")
- },
- {.name = NULL}
+ {.name = "help",
+ .data = N_("edit XML for a domain's managed save state file")
+ },
+ {.name = "desc",
+ .data = N_("Edit the domain XML associated with the managed save state
file")
+ },
+ {.name = NULL}
};
static const vshCmdOptDef opts_managed_save_edit[] = {
--
2.20.1
3:33 p.m.
New subject: [libvirt] [PATCH 01/14] virsh: fix indentation of info_managed_save_edit
On Mon, Apr 01, 2019 at 09:33:18 +0200, Ján Tomko wrote:
Use four spaces instead of three.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
tools/virsh-domain.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
ACK
3:33 p.m.
New subject: [libvirt] [PATCH 02/14] virsh-completer: fix typo
Use the posessive determiner instead of a contracted auxiliary.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
tools/virsh-completer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c
index e9ef9b99f9..760ed2088f 100644
--- a/tools/virsh-completer.c
+++ b/tools/virsh-completer.c
@@ -58,7 +58,7 @@
*
* The @flags contains a .completer_flags value defined for each
* use or 0 if no .completer_flags were specified. If a completer
- * is generic enough @flags can be used to alter it's behaviour.
+ * is generic enough @flags can be used to alter its behaviour.
* For instance, a completer to fetch names of domains can use
* @flags to return names of only domains in a particular state
* that the command accepts.
--
2.20.1
3:34 p.m.
New subject: [libvirt] [PATCH 02/14] virsh-completer: fix typo
On Mon, Apr 01, 2019 at 09:33:19 +0200, Ján Tomko wrote:
Use the posessive determiner instead of a contracted auxiliary.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
tools/virsh-completer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
ACK
3:33 p.m.
New subject: [libvirt] [PATCH 03/14] virsh-completer: reword documentation
Use less strict wording that does not threaten the reader with
circumstances.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
tools/virsh-completer.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c
index 760ed2088f..464a2751fa 100644
--- a/tools/virsh-completer.c
+++ b/tools/virsh-completer.c
@@ -63,9 +63,9 @@
* @flags to return names of only domains in a particular state
* that the command accepts.
*
- * Under no circumstances should a completer output anything.
- * Neither to stdout nor to stderr. This would harm the user
- * experience.
+ * A completer may not harm the user experience by outputting
+ * anything or, through inaction, allow the user experience
+ * to be harmed by output.
*/
--
2.20.1
3:34 p.m.
New subject: [libvirt] [PATCH 03/14] virsh-completer: reword documentation
On Mon, Apr 01, 2019 at 09:33:20 +0200, Ján Tomko wrote:
Use less strict wording that does not threaten the reader with
circumstances.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
tools/virsh-completer.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
NACK
3:33 p.m.
New subject: [libvirt] [PATCH 04/14] virsh-completer: switch to using tmp instead of ret
Construct the potential return value in an array called 'tmp'
and only assign it to 'ret' if we're going to return it.
This will allow us to unify the error and success paths.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
tools/virsh-completer.c | 195 +++++++++++++++++++++++++---------------
1 file changed, 125 insertions(+), 70 deletions(-)
diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c
index 464a2751fa..0750945b84 100644
--- a/tools/virsh-completer.c
+++ b/tools/virsh-completer.c
@@ -79,6 +79,7 @@ virshDomainNameCompleter(vshControl *ctl,
int ndomains = 0;
size_t i = 0;
char **ret = NULL;
+ char **tmp = NULL;
virCheckFlags(VIR_CONNECT_LIST_DOMAINS_ACTIVE |
VIR_CONNECT_LIST_DOMAINS_INACTIVE |
@@ -95,19 +96,21 @@ virshDomainNameCompleter(vshControl *ctl,
if ((ndomains = virConnectListAllDomains(priv->conn, &domains, flags)) <
0)
return NULL;
- if (VIR_ALLOC_N(ret, ndomains + 1) < 0)
+ if (VIR_ALLOC_N(tmp, ndomains + 1) < 0)
goto error;
for (i = 0; i < ndomains; i++) {
const char *name = virDomainGetName(domains[i]);
- if (VIR_STRDUP(ret[i], name) < 0)
+ if (VIR_STRDUP(tmp[i], name) < 0)
goto error;
virshDomainFree(domains[i]);
}
VIR_FREE(domains);
+ VIR_STEAL_PTR(ret, tmp);
+
return ret;
error:
@@ -115,8 +118,8 @@ virshDomainNameCompleter(vshControl *ctl,
virshDomainFree(domains[i]);
VIR_FREE(domains);
for (i = 0; i < ndomains; i++)
- VIR_FREE(ret[i]);
- VIR_FREE(ret);
+ VIR_FREE(tmp[i]);
+ VIR_FREE(tmp);
return NULL;
}
@@ -232,6 +235,7 @@ virshStoragePoolNameCompleter(vshControl *ctl,
int npools = 0;
size_t i = 0;
char **ret = NULL;
+ char **tmp = NULL;
virCheckFlags(VIR_CONNECT_LIST_STORAGE_POOLS_INACTIVE |
VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE |
@@ -244,19 +248,21 @@ virshStoragePoolNameCompleter(vshControl *ctl,
if ((npools = virConnectListAllStoragePools(priv->conn, &pools, flags)) <
0)
return NULL;
- if (VIR_ALLOC_N(ret, npools + 1) < 0)
+ if (VIR_ALLOC_N(tmp, npools + 1) < 0)
goto error;
for (i = 0; i < npools; i++) {
const char *name = virStoragePoolGetName(pools[i]);
- if (VIR_STRDUP(ret[i], name) < 0)
+ if (VIR_STRDUP(tmp[i], name) < 0)
goto error;
virStoragePoolFree(pools[i]);
}
VIR_FREE(pools);
+ VIR_STEAL_PTR(ret, tmp);
+
return ret;
error:
@@ -264,8 +270,8 @@ virshStoragePoolNameCompleter(vshControl *ctl,
virStoragePoolFree(pools[i]);
VIR_FREE(pools);
for (i = 0; i < npools; i++)
- VIR_FREE(ret[i]);
- VIR_FREE(ret);
+ VIR_FREE(tmp[i]);
+ VIR_FREE(tmp);
return NULL;
}
@@ -282,6 +288,7 @@ virshStorageVolNameCompleter(vshControl *ctl,
int nvols = 0;
size_t i = 0;
char **ret = NULL;
+ char **tmp = NULL;
virCheckFlags(0, NULL);
@@ -295,20 +302,23 @@ virshStorageVolNameCompleter(vshControl *ctl,
goto error;
nvols = rc;
- if (VIR_ALLOC_N(ret, nvols + 1) < 0)
+ if (VIR_ALLOC_N(tmp, nvols + 1) < 0)
goto error;
for (i = 0; i < nvols; i++) {
const char *name = virStorageVolGetName(vols[i]);
- if (VIR_STRDUP(ret[i], name) < 0)
+ if (VIR_STRDUP(tmp[i], name) < 0)
goto error;
virStorageVolFree(vols[i]);
}
+
VIR_FREE(vols);
virStoragePoolFree(pool);
+ VIR_STEAL_PTR(ret, tmp);
+
return ret;
error:
@@ -316,8 +326,8 @@ virshStorageVolNameCompleter(vshControl *ctl,
virStorageVolFree(vols[i]);
VIR_FREE(vols);
for (i = 0; i < nvols; i++)
- VIR_FREE(ret[i]);
- VIR_FREE(ret);
+ VIR_FREE(tmp[i]);
+ VIR_FREE(tmp);
virStoragePoolFree(pool);
return NULL;
}
@@ -333,6 +343,7 @@ virshInterfaceNameCompleter(vshControl *ctl,
int nifaces = 0;
size_t i = 0;
char **ret = NULL;
+ char **tmp = NULL;
virCheckFlags(VIR_CONNECT_LIST_INTERFACES_ACTIVE |
VIR_CONNECT_LIST_INTERFACES_INACTIVE,
@@ -344,19 +355,21 @@ virshInterfaceNameCompleter(vshControl *ctl,
if ((nifaces = virConnectListAllInterfaces(priv->conn, &ifaces, flags)) <
0)
return NULL;
- if (VIR_ALLOC_N(ret, nifaces + 1) < 0)
+ if (VIR_ALLOC_N(tmp, nifaces + 1) < 0)
goto error;
for (i = 0; i < nifaces; i++) {
const char *name = virInterfaceGetName(ifaces[i]);
- if (VIR_STRDUP(ret[i], name) < 0)
+ if (VIR_STRDUP(tmp[i], name) < 0)
goto error;
virInterfaceFree(ifaces[i]);
}
VIR_FREE(ifaces);
+ VIR_STEAL_PTR(ret, tmp);
+
return ret;
error:
@@ -364,8 +377,8 @@ virshInterfaceNameCompleter(vshControl *ctl,
virInterfaceFree(ifaces[i]);
VIR_FREE(ifaces);
for (i = 0; i < nifaces; i++)
- VIR_FREE(ret[i]);
- VIR_FREE(ret);
+ VIR_FREE(tmp[i]);
+ VIR_FREE(tmp);
return NULL;
}
@@ -380,6 +393,7 @@ virshNetworkNameCompleter(vshControl *ctl,
int nnets = 0;
size_t i = 0;
char **ret = NULL;
+ char **tmp = NULL;
virCheckFlags(VIR_CONNECT_LIST_NETWORKS_INACTIVE |
VIR_CONNECT_LIST_NETWORKS_ACTIVE |
@@ -392,19 +406,21 @@ virshNetworkNameCompleter(vshControl *ctl,
if ((nnets = virConnectListAllNetworks(priv->conn, &nets, flags)) < 0)
return NULL;
- if (VIR_ALLOC_N(ret, nnets + 1) < 0)
+ if (VIR_ALLOC_N(tmp, nnets + 1) < 0)
goto error;
for (i = 0; i < nnets; i++) {
const char *name = virNetworkGetName(nets[i]);
- if (VIR_STRDUP(ret[i], name) < 0)
+ if (VIR_STRDUP(tmp[i], name) < 0)
goto error;
virNetworkFree(nets[i]);
}
VIR_FREE(nets);
+ VIR_STEAL_PTR(ret, tmp);
+
return ret;
error:
@@ -412,8 +428,8 @@ virshNetworkNameCompleter(vshControl *ctl,
virNetworkFree(nets[i]);
VIR_FREE(nets);
for (i = 0; i < nnets; i++)
- VIR_FREE(ret[i]);
- VIR_FREE(ret);
+ VIR_FREE(tmp[i]);
+ VIR_FREE(tmp);
return NULL;
}
@@ -425,21 +441,24 @@ virshNetworkEventNameCompleter(vshControl *ctl ATTRIBUTE_UNUSED,
{
size_t i = 0;
char **ret = NULL;
+ char **tmp = NULL;
virCheckFlags(0, NULL);
- if (VIR_ALLOC_N(ret, VIR_NETWORK_EVENT_ID_LAST + 1) < 0)
+ if (VIR_ALLOC_N(tmp, VIR_NETWORK_EVENT_ID_LAST + 1) < 0)
goto error;
for (i = 0; i < VIR_NETWORK_EVENT_ID_LAST; i++) {
- if (VIR_STRDUP(ret[i], virshNetworkEventCallbacks[i].name) < 0)
+ if (VIR_STRDUP(tmp[i], virshNetworkEventCallbacks[i].name) < 0)
goto error;
}
+ VIR_STEAL_PTR(ret, tmp);
+
return ret;
error:
- virStringListFree(ret);
+ virStringListFree(tmp);
return NULL;
}
@@ -454,6 +473,7 @@ virshNodeDeviceNameCompleter(vshControl *ctl,
int ndevs = 0;
size_t i = 0;
char **ret = NULL;
+ char **tmp = NULL;
virCheckFlags(0, NULL);
@@ -463,19 +483,21 @@ virshNodeDeviceNameCompleter(vshControl *ctl,
if ((ndevs = virConnectListAllNodeDevices(priv->conn, &devs, flags)) < 0)
return NULL;
- if (VIR_ALLOC_N(ret, ndevs + 1) < 0)
+ if (VIR_ALLOC_N(tmp, ndevs + 1) < 0)
goto error;
for (i = 0; i < ndevs; i++) {
const char *name = virNodeDeviceGetName(devs[i]);
- if (VIR_STRDUP(ret[i], name) < 0)
+ if (VIR_STRDUP(tmp[i], name) < 0)
goto error;
virNodeDeviceFree(devs[i]);
}
VIR_FREE(devs);
+ VIR_STEAL_PTR(ret, tmp);
+
return ret;
error:
@@ -483,8 +505,8 @@ virshNodeDeviceNameCompleter(vshControl *ctl,
virNodeDeviceFree(devs[i]);
VIR_FREE(devs);
for (i = 0; i < ndevs; i++)
- VIR_FREE(ret[i]);
- VIR_FREE(ret);
+ VIR_FREE(tmp[i]);
+ VIR_FREE(tmp);
return NULL;
}
@@ -499,6 +521,7 @@ virshNWFilterNameCompleter(vshControl *ctl,
int nnwfilters = 0;
size_t i = 0;
char **ret = NULL;
+ char **tmp = NULL;
virCheckFlags(0, NULL);
@@ -508,19 +531,21 @@ virshNWFilterNameCompleter(vshControl *ctl,
if ((nnwfilters = virConnectListAllNWFilters(priv->conn, &nwfilters, flags))
< 0)
return NULL;
- if (VIR_ALLOC_N(ret, nnwfilters + 1) < 0)
+ if (VIR_ALLOC_N(tmp, nnwfilters + 1) < 0)
goto error;
for (i = 0; i < nnwfilters; i++) {
const char *name = virNWFilterGetName(nwfilters[i]);
- if (VIR_STRDUP(ret[i], name) < 0)
+ if (VIR_STRDUP(tmp[i], name) < 0)
goto error;
virNWFilterFree(nwfilters[i]);
}
VIR_FREE(nwfilters);
+ VIR_STEAL_PTR(ret, tmp);
+
return ret;
error:
@@ -528,8 +553,8 @@ virshNWFilterNameCompleter(vshControl *ctl,
virNWFilterFree(nwfilters[i]);
VIR_FREE(nwfilters);
for (i = 0; i < nnwfilters; i++)
- VIR_FREE(ret[i]);
- VIR_FREE(ret);
+ VIR_FREE(tmp[i]);
+ VIR_FREE(tmp);
return NULL;
}
@@ -544,6 +569,7 @@ virshNWFilterBindingNameCompleter(vshControl *ctl,
int nbindings = 0;
size_t i = 0;
char **ret = NULL;
+ char **tmp = NULL;
virCheckFlags(0, NULL);
@@ -553,19 +579,21 @@ virshNWFilterBindingNameCompleter(vshControl *ctl,
if ((nbindings = virConnectListAllNWFilterBindings(priv->conn, &bindings,
flags)) < 0)
return NULL;
- if (VIR_ALLOC_N(ret, nbindings + 1) < 0)
+ if (VIR_ALLOC_N(tmp, nbindings + 1) < 0)
goto error;
for (i = 0; i < nbindings; i++) {
const char *name = virNWFilterBindingGetPortDev(bindings[i]);
- if (VIR_STRDUP(ret[i], name) < 0)
+ if (VIR_STRDUP(tmp[i], name) < 0)
goto error;
virNWFilterBindingFree(bindings[i]);
}
VIR_FREE(bindings);
+ VIR_STEAL_PTR(ret, tmp);
+
return ret;
error:
@@ -573,8 +601,8 @@ virshNWFilterBindingNameCompleter(vshControl *ctl,
virNWFilterBindingFree(bindings[i]);
VIR_FREE(bindings);
for (i = 0; i < nbindings; i++)
- VIR_FREE(ret[i]);
- VIR_FREE(ret);
+ VIR_FREE(tmp[i]);
+ VIR_FREE(tmp);
return NULL;
}
@@ -589,6 +617,7 @@ virshSecretUUIDCompleter(vshControl *ctl,
int nsecrets = 0;
size_t i = 0;
char **ret = NULL;
+ char **tmp = NULL;
virCheckFlags(0, NULL);
@@ -598,20 +627,22 @@ virshSecretUUIDCompleter(vshControl *ctl,
if ((nsecrets = virConnectListAllSecrets(priv->conn, &secrets, flags)) <
0)
return NULL;
- if (VIR_ALLOC_N(ret, nsecrets + 1) < 0)
+ if (VIR_ALLOC_N(tmp, nsecrets + 1) < 0)
goto error;
for (i = 0; i < nsecrets; i++) {
char uuid[VIR_UUID_STRING_BUFLEN];
if (virSecretGetUUIDString(secrets[i], uuid) < 0 ||
- VIR_STRDUP(ret[i], uuid) < 0)
+ VIR_STRDUP(tmp[i], uuid) < 0)
goto error;
virSecretFree(secrets[i]);
}
VIR_FREE(secrets);
+ VIR_STEAL_PTR(ret, tmp);
+
return ret;
error:
@@ -619,8 +650,8 @@ virshSecretUUIDCompleter(vshControl *ctl,
virSecretFree(secrets[i]);
VIR_FREE(secrets);
for (i = 0; i < nsecrets; i++)
- VIR_FREE(ret[i]);
- VIR_FREE(ret);
+ VIR_FREE(tmp[i]);
+ VIR_FREE(tmp);
return NULL;
}
@@ -637,6 +668,7 @@ virshSnapshotNameCompleter(vshControl *ctl,
int nsnapshots = 0;
size_t i = 0;
char **ret = NULL;
+ char **tmp = NULL;
virCheckFlags(0, NULL);
@@ -650,13 +682,13 @@ virshSnapshotNameCompleter(vshControl *ctl,
goto error;
nsnapshots = rc;
- if (VIR_ALLOC_N(ret, nsnapshots + 1) < 0)
+ if (VIR_ALLOC_N(tmp, nsnapshots + 1) < 0)
goto error;
for (i = 0; i < nsnapshots; i++) {
const char *name = virDomainSnapshotGetName(snapshots[i]);
- if (VIR_STRDUP(ret[i], name) < 0)
+ if (VIR_STRDUP(tmp[i], name) < 0)
goto error;
virshDomainSnapshotFree(snapshots[i]);
@@ -664,6 +696,8 @@ virshSnapshotNameCompleter(vshControl *ctl,
VIR_FREE(snapshots);
virshDomainFree(dom);
+ VIR_STEAL_PTR(ret, tmp);
+
return ret;
error:
@@ -671,8 +705,8 @@ virshSnapshotNameCompleter(vshControl *ctl,
virshDomainSnapshotFree(snapshots[i]);
VIR_FREE(snapshots);
for (i = 0; i < nsnapshots; i++)
- VIR_FREE(ret[i]);
- VIR_FREE(ret);
+ VIR_FREE(tmp[i]);
+ VIR_FREE(tmp);
virshDomainFree(dom);
return NULL;
}
@@ -698,6 +732,7 @@ virshAllocpagesPagesizeCompleter(vshControl *ctl,
char *cap_xml = NULL;
char **ret = NULL;
char *unit = NULL;
+ char **tmp = NULL;
virCheckFlags(0, NULL);
@@ -724,7 +759,7 @@ virshAllocpagesPagesizeCompleter(vshControl *ctl,
if (npages <= 0)
goto error;
- if (VIR_ALLOC_N(ret, npages + 1) < 0)
+ if (VIR_ALLOC_N(tmp, npages + 1) < 0)
goto error;
for (i = 0; i < npages; i++) {
@@ -737,10 +772,12 @@ virshAllocpagesPagesizeCompleter(vshControl *ctl,
if (virScaleInteger(&byteval, unit, 1024, UINT_MAX) < 0)
goto error;
size = vshPrettyCapacity(byteval, &suffix);
- if (virAsprintf(&ret[i], "%.0f%s", size, suffix) < 0)
+ if (virAsprintf(&tmp[i], "%.0f%s", size, suffix) < 0)
goto error;
}
+ VIR_STEAL_PTR(ret, tmp);
+
cleanup:
xmlXPathFreeContext(ctxt);
VIR_FREE(pages);
@@ -753,11 +790,11 @@ virshAllocpagesPagesizeCompleter(vshControl *ctl,
return ret;
error:
- if (ret) {
+ if (tmp) {
for (i = 0; i < npages; i++)
- VIR_FREE(ret[i]);
+ VIR_FREE(tmp[i]);
}
- VIR_FREE(ret);
+ VIR_FREE(tmp);
goto cleanup;
}
@@ -769,21 +806,24 @@ virshSecretEventNameCompleter(vshControl *ctl ATTRIBUTE_UNUSED,
{
size_t i;
char **ret = NULL;
+ char **tmp = NULL;
virCheckFlags(0, NULL);
- if (VIR_ALLOC_N(ret, VIR_SECRET_EVENT_ID_LAST + 1) < 0)
+ if (VIR_ALLOC_N(tmp, VIR_SECRET_EVENT_ID_LAST + 1) < 0)
goto error;
for (i = 0; i < VIR_SECRET_EVENT_ID_LAST; i++) {
- if (VIR_STRDUP(ret[i], virshSecretEventCallbacks[i].name) < 0)
+ if (VIR_STRDUP(tmp[i], virshSecretEventCallbacks[i].name) < 0)
goto error;
}
+ VIR_STEAL_PTR(ret, tmp);
+
return ret;
error:
- virStringListFree(ret);
+ virStringListFree(tmp);
return NULL;
}
@@ -795,21 +835,24 @@ virshDomainEventNameCompleter(vshControl *ctl ATTRIBUTE_UNUSED,
{
size_t i = 0;
char **ret = NULL;
+ char **tmp = NULL;
virCheckFlags(0, NULL);
- if (VIR_ALLOC_N(ret, VIR_DOMAIN_EVENT_ID_LAST + 1) < 0)
+ if (VIR_ALLOC_N(tmp, VIR_DOMAIN_EVENT_ID_LAST + 1) < 0)
goto error;
for (i = 0; i < VIR_DOMAIN_EVENT_ID_LAST; i++) {
- if (VIR_STRDUP(ret[i], virshDomainEventCallbacks[i].name) < 0)
+ if (VIR_STRDUP(tmp[i], virshDomainEventCallbacks[i].name) < 0)
goto error;
}
+ VIR_STEAL_PTR(ret, tmp);
+
return ret;
error:
- virStringListFree(ret);
+ virStringListFree(tmp);
return NULL;
}
@@ -821,21 +864,24 @@ virshPoolEventNameCompleter(vshControl *ctl ATTRIBUTE_UNUSED,
{
size_t i = 0;
char **ret = NULL;
+ char **tmp = NULL;
virCheckFlags(0, NULL);
- if (VIR_ALLOC_N(ret, VIR_STORAGE_POOL_EVENT_ID_LAST + 1) < 0)
+ if (VIR_ALLOC_N(tmp, VIR_STORAGE_POOL_EVENT_ID_LAST + 1) < 0)
goto error;
for (i = 0; i < VIR_STORAGE_POOL_EVENT_ID_LAST; i++) {
- if (VIR_STRDUP(ret[i], virshPoolEventCallbacks[i].name) < 0)
+ if (VIR_STRDUP(tmp[i], virshPoolEventCallbacks[i].name) < 0)
goto error;
}
+ VIR_STEAL_PTR(ret, tmp);
+
return ret;
error:
- virStringListFree(ret);
+ virStringListFree(tmp);
return NULL;
}
@@ -856,6 +902,7 @@ virshDomainInterfaceStateCompleter(vshControl *ctl,
xmlNodePtr *interfaces = NULL;
char *xpath = NULL;
char *state = NULL;
+ char **tmp = NULL;
virCheckFlags(0, NULL);
@@ -885,18 +932,20 @@ virshDomainInterfaceStateCompleter(vshControl *ctl,
ctxt->node = interfaces[0];
- if (VIR_ALLOC_N(ret, 2) < 0)
+ if (VIR_ALLOC_N(tmp, 2) < 0)
goto error;
if ((state = virXPathString("string(./link/@state)", ctxt)) &&
STREQ(state, "down")) {
- if (VIR_STRDUP(ret[0], "up") < 0)
+ if (VIR_STRDUP(tmp[0], "up") < 0)
goto error;
} else {
- if (VIR_STRDUP(ret[0], "down") < 0)
+ if (VIR_STRDUP(tmp[0], "down") < 0)
goto error;
}
+ VIR_STEAL_PTR(ret, tmp);
+
cleanup:
VIR_FREE(state);
VIR_FREE(xpath);
@@ -906,8 +955,8 @@ virshDomainInterfaceStateCompleter(vshControl *ctl,
return ret;
error:
- virStringListFree(ret);
- ret = NULL;
+ virStringListFree(tmp);
+ tmp = NULL;
goto cleanup;
}
@@ -919,21 +968,24 @@ virshNodedevEventNameCompleter(vshControl *ctl ATTRIBUTE_UNUSED,
{
size_t i = 0;
char **ret = NULL;
+ char **tmp = NULL;
virCheckFlags(0, NULL);
- if (VIR_ALLOC_N(ret, VIR_NODE_DEVICE_EVENT_ID_LAST + 1) < 0)
+ if (VIR_ALLOC_N(tmp, VIR_NODE_DEVICE_EVENT_ID_LAST + 1) < 0)
goto error;
for (i = 0; i < VIR_NODE_DEVICE_EVENT_ID_LAST; i++) {
- if (VIR_STRDUP(ret[i], virshNodedevEventCallbacks[i].name) < 0)
+ if (VIR_STRDUP(tmp[i], virshNodedevEventCallbacks[i].name) < 0)
goto error;
}
+ VIR_STEAL_PTR(ret, tmp);
+
return ret;
error:
- virStringListFree(ret);
+ virStringListFree(tmp);
return NULL;
}
@@ -951,6 +1003,7 @@ virshCellnoCompleter(vshControl *ctl,
size_t i = 0;
char *cap_xml = NULL;
char **ret = NULL;
+ char **tmp = NULL;
virCheckFlags(0, NULL);
@@ -967,14 +1020,16 @@ virshCellnoCompleter(vshControl *ctl,
if (ncells <= 0)
goto error;
- if (VIR_ALLOC_N(ret, ncells + 1))
+ if (VIR_ALLOC_N(tmp, ncells + 1))
goto error;
for (i = 0; i < ncells; i++) {
- if (!(ret[i] = virXMLPropString(cells[i], "id")))
+ if (!(tmp[i] = virXMLPropString(cells[i], "id")))
goto error;
}
+ VIR_STEAL_PTR(ret, tmp);
+
cleanup:
xmlXPathFreeContext(ctxt);
VIR_FREE(cells);
@@ -984,11 +1039,11 @@ virshCellnoCompleter(vshControl *ctl,
return ret;
error:
- if (ret) {
+ if (tmp) {
for (i = 0; i < ncells; i++)
- VIR_FREE(ret[i]);
+ VIR_FREE(tmp[i]);
}
- VIR_FREE(ret);
+ VIR_FREE(tmp);
goto cleanup;
}
--
2.20.1
3:40 p.m.
New subject: [libvirt] [PATCH 04/14] virsh-completer: switch to using tmp instead of ret
On Mon, Apr 01, 2019 at 09:33:21 +0200, Ján Tomko wrote:
Construct the potential return value in an array called
'tmp'
and only assign it to 'ret' if we're going to return it.
This will allow us to unify the error and success paths.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
tools/virsh-completer.c | 195 +++++++++++++++++++++++++---------------
1 file changed, 125 insertions(+), 70 deletions(-)
There's at least 1 unrelated whitespace change.
ACK regardless.
3:33 p.m.
New subject: [libvirt] [PATCH 05/14] virsh-completer: add a cleanup label everywhere
Unify the cleanup paths for error and success.
Now that 'ret' is only set (from tmp) on the success path,
it is safe to jump right before 'return ret' after processing
the error block.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
tools/virsh-completer.c | 45 +++++++++++++++++++++++++++--------------
1 file changed, 30 insertions(+), 15 deletions(-)
diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c
index 0750945b84..46fedc06d9 100644
--- a/tools/virsh-completer.c
+++ b/tools/virsh-completer.c
@@ -111,6 +111,7 @@ virshDomainNameCompleter(vshControl *ctl,
VIR_STEAL_PTR(ret, tmp);
+ cleanup:
return ret;
error:
@@ -120,7 +121,7 @@ virshDomainNameCompleter(vshControl *ctl,
for (i = 0; i < ndomains; i++)
VIR_FREE(tmp[i]);
VIR_FREE(tmp);
- return NULL;
+ goto cleanup;
}
@@ -263,6 +264,7 @@ virshStoragePoolNameCompleter(vshControl *ctl,
VIR_STEAL_PTR(ret, tmp);
+ cleanup:
return ret;
error:
@@ -272,7 +274,7 @@ virshStoragePoolNameCompleter(vshControl *ctl,
for (i = 0; i < npools; i++)
VIR_FREE(tmp[i]);
VIR_FREE(tmp);
- return NULL;
+ goto cleanup;
}
@@ -319,6 +321,7 @@ virshStorageVolNameCompleter(vshControl *ctl,
VIR_STEAL_PTR(ret, tmp);
+ cleanup:
return ret;
error:
@@ -329,7 +332,7 @@ virshStorageVolNameCompleter(vshControl *ctl,
VIR_FREE(tmp[i]);
VIR_FREE(tmp);
virStoragePoolFree(pool);
- return NULL;
+ goto cleanup;
}
@@ -370,6 +373,7 @@ virshInterfaceNameCompleter(vshControl *ctl,
VIR_STEAL_PTR(ret, tmp);
+ cleanup:
return ret;
error:
@@ -379,7 +383,7 @@ virshInterfaceNameCompleter(vshControl *ctl,
for (i = 0; i < nifaces; i++)
VIR_FREE(tmp[i]);
VIR_FREE(tmp);
- return NULL;
+ goto cleanup;
}
@@ -421,6 +425,7 @@ virshNetworkNameCompleter(vshControl *ctl,
VIR_STEAL_PTR(ret, tmp);
+ cleanup:
return ret;
error:
@@ -430,7 +435,7 @@ virshNetworkNameCompleter(vshControl *ctl,
for (i = 0; i < nnets; i++)
VIR_FREE(tmp[i]);
VIR_FREE(tmp);
- return NULL;
+ goto cleanup;
}
@@ -455,11 +460,12 @@ virshNetworkEventNameCompleter(vshControl *ctl ATTRIBUTE_UNUSED,
VIR_STEAL_PTR(ret, tmp);
+ cleanup:
return ret;
error:
virStringListFree(tmp);
- return NULL;
+ goto cleanup;
}
@@ -498,6 +504,7 @@ virshNodeDeviceNameCompleter(vshControl *ctl,
VIR_STEAL_PTR(ret, tmp);
+ cleanup:
return ret;
error:
@@ -507,7 +514,7 @@ virshNodeDeviceNameCompleter(vshControl *ctl,
for (i = 0; i < ndevs; i++)
VIR_FREE(tmp[i]);
VIR_FREE(tmp);
- return NULL;
+ goto cleanup;
}
@@ -546,6 +553,7 @@ virshNWFilterNameCompleter(vshControl *ctl,
VIR_STEAL_PTR(ret, tmp);
+ cleanup:
return ret;
error:
@@ -555,7 +563,7 @@ virshNWFilterNameCompleter(vshControl *ctl,
for (i = 0; i < nnwfilters; i++)
VIR_FREE(tmp[i]);
VIR_FREE(tmp);
- return NULL;
+ goto cleanup;
}
@@ -594,6 +602,7 @@ virshNWFilterBindingNameCompleter(vshControl *ctl,
VIR_STEAL_PTR(ret, tmp);
+ cleanup:
return ret;
error:
@@ -603,7 +612,7 @@ virshNWFilterBindingNameCompleter(vshControl *ctl,
for (i = 0; i < nbindings; i++)
VIR_FREE(tmp[i]);
VIR_FREE(tmp);
- return NULL;
+ goto cleanup;
}
@@ -643,6 +652,7 @@ virshSecretUUIDCompleter(vshControl *ctl,
VIR_STEAL_PTR(ret, tmp);
+ cleanup:
return ret;
error:
@@ -652,7 +662,7 @@ virshSecretUUIDCompleter(vshControl *ctl,
for (i = 0; i < nsecrets; i++)
VIR_FREE(tmp[i]);
VIR_FREE(tmp);
- return NULL;
+ goto cleanup;
}
@@ -698,6 +708,7 @@ virshSnapshotNameCompleter(vshControl *ctl,
VIR_STEAL_PTR(ret, tmp);
+ cleanup:
return ret;
error:
@@ -708,7 +719,7 @@ virshSnapshotNameCompleter(vshControl *ctl,
VIR_FREE(tmp[i]);
VIR_FREE(tmp);
virshDomainFree(dom);
- return NULL;
+ goto cleanup;
}
char **
@@ -820,11 +831,12 @@ virshSecretEventNameCompleter(vshControl *ctl ATTRIBUTE_UNUSED,
VIR_STEAL_PTR(ret, tmp);
+ cleanup:
return ret;
error:
virStringListFree(tmp);
- return NULL;
+ goto cleanup;
}
@@ -849,11 +861,12 @@ virshDomainEventNameCompleter(vshControl *ctl ATTRIBUTE_UNUSED,
VIR_STEAL_PTR(ret, tmp);
+ cleanup:
return ret;
error:
virStringListFree(tmp);
- return NULL;
+ goto cleanup;
}
@@ -878,11 +891,12 @@ virshPoolEventNameCompleter(vshControl *ctl ATTRIBUTE_UNUSED,
VIR_STEAL_PTR(ret, tmp);
+ cleanup:
return ret;
error:
virStringListFree(tmp);
- return NULL;
+ goto cleanup;
}
@@ -982,11 +996,12 @@ virshNodedevEventNameCompleter(vshControl *ctl ATTRIBUTE_UNUSED,
VIR_STEAL_PTR(ret, tmp);
+ cleanup:
return ret;
error:
virStringListFree(tmp);
- return NULL;
+ goto cleanup;
}
--
2.20.1
3:33 p.m.
New subject: [libvirt] [PATCH 06/14] virsh-completer: unify cleanup of items in name completers
Merge the cleanup of fetched items for the success and the error
paths.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
tools/virsh-completer.c | 97 ++++++++++++++---------------------------
1 file changed, 32 insertions(+), 65 deletions(-)
diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c
index 46fedc06d9..9d56659259 100644
--- a/tools/virsh-completer.c
+++ b/tools/virsh-completer.c
@@ -104,20 +104,17 @@ virshDomainNameCompleter(vshControl *ctl,
if (VIR_STRDUP(tmp[i], name) < 0)
goto error;
-
- virshDomainFree(domains[i]);
}
- VIR_FREE(domains);
VIR_STEAL_PTR(ret, tmp);
cleanup:
+ for (i = 0; i < ndomains; i++)
+ virshDomainFree(domains[i]);
+ VIR_FREE(domains);
return ret;
error:
- for (; i < ndomains; i++)
- virshDomainFree(domains[i]);
- VIR_FREE(domains);
for (i = 0; i < ndomains; i++)
VIR_FREE(tmp[i]);
VIR_FREE(tmp);
@@ -257,20 +254,17 @@ virshStoragePoolNameCompleter(vshControl *ctl,
if (VIR_STRDUP(tmp[i], name) < 0)
goto error;
-
- virStoragePoolFree(pools[i]);
}
- VIR_FREE(pools);
VIR_STEAL_PTR(ret, tmp);
cleanup:
+ for (i = 0; i < npools; i++)
+ virStoragePoolFree(pools[i]);
+ VIR_FREE(pools);
return ret;
error:
- for (; i < npools; i++)
- virStoragePoolFree(pools[i]);
- VIR_FREE(pools);
for (i = 0; i < npools; i++)
VIR_FREE(tmp[i]);
VIR_FREE(tmp);
@@ -312,26 +306,21 @@ virshStorageVolNameCompleter(vshControl *ctl,
if (VIR_STRDUP(tmp[i], name) < 0)
goto error;
-
- virStorageVolFree(vols[i]);
}
- VIR_FREE(vols);
- virStoragePoolFree(pool);
-
VIR_STEAL_PTR(ret, tmp);
cleanup:
+ virStoragePoolFree(pool);
+ for (i = 0; i < nvols; i++)
+ virStorageVolFree(vols[i]);
+ VIR_FREE(vols);
return ret;
error:
- for (; i < nvols; i++)
- virStorageVolFree(vols[i]);
- VIR_FREE(vols);
for (i = 0; i < nvols; i++)
VIR_FREE(tmp[i]);
VIR_FREE(tmp);
- virStoragePoolFree(pool);
goto cleanup;
}
@@ -366,20 +355,17 @@ virshInterfaceNameCompleter(vshControl *ctl,
if (VIR_STRDUP(tmp[i], name) < 0)
goto error;
-
- virInterfaceFree(ifaces[i]);
}
- VIR_FREE(ifaces);
VIR_STEAL_PTR(ret, tmp);
cleanup:
+ for (i = 0; i < nifaces; i++)
+ virInterfaceFree(ifaces[i]);
+ VIR_FREE(ifaces);
return ret;
error:
- for (; i < nifaces; i++)
- virInterfaceFree(ifaces[i]);
- VIR_FREE(ifaces);
for (i = 0; i < nifaces; i++)
VIR_FREE(tmp[i]);
VIR_FREE(tmp);
@@ -418,20 +404,17 @@ virshNetworkNameCompleter(vshControl *ctl,
if (VIR_STRDUP(tmp[i], name) < 0)
goto error;
-
- virNetworkFree(nets[i]);
}
- VIR_FREE(nets);
VIR_STEAL_PTR(ret, tmp);
cleanup:
+ for (i = 0; i < nnets; i++)
+ virNetworkFree(nets[i]);
+ VIR_FREE(nets);
return ret;
error:
- for (; i < nnets; i++)
- virNetworkFree(nets[i]);
- VIR_FREE(nets);
for (i = 0; i < nnets; i++)
VIR_FREE(tmp[i]);
VIR_FREE(tmp);
@@ -497,20 +480,17 @@ virshNodeDeviceNameCompleter(vshControl *ctl,
if (VIR_STRDUP(tmp[i], name) < 0)
goto error;
-
- virNodeDeviceFree(devs[i]);
}
- VIR_FREE(devs);
VIR_STEAL_PTR(ret, tmp);
cleanup:
+ for (i = 0; i < ndevs; i++)
+ virNodeDeviceFree(devs[i]);
+ VIR_FREE(devs);
return ret;
error:
- for (; i < ndevs; i++)
- virNodeDeviceFree(devs[i]);
- VIR_FREE(devs);
for (i = 0; i < ndevs; i++)
VIR_FREE(tmp[i]);
VIR_FREE(tmp);
@@ -546,20 +526,17 @@ virshNWFilterNameCompleter(vshControl *ctl,
if (VIR_STRDUP(tmp[i], name) < 0)
goto error;
-
- virNWFilterFree(nwfilters[i]);
}
- VIR_FREE(nwfilters);
VIR_STEAL_PTR(ret, tmp);
cleanup:
+ for (i = 0; i < nnwfilters; i++)
+ virNWFilterFree(nwfilters[i]);
+ VIR_FREE(nwfilters);
return ret;
error:
- for (; i < nnwfilters; i++)
- virNWFilterFree(nwfilters[i]);
- VIR_FREE(nwfilters);
for (i = 0; i < nnwfilters; i++)
VIR_FREE(tmp[i]);
VIR_FREE(tmp);
@@ -595,20 +572,17 @@ virshNWFilterBindingNameCompleter(vshControl *ctl,
if (VIR_STRDUP(tmp[i], name) < 0)
goto error;
-
- virNWFilterBindingFree(bindings[i]);
}
- VIR_FREE(bindings);
VIR_STEAL_PTR(ret, tmp);
cleanup:
+ for (i = 0; i < nbindings; i++)
+ virNWFilterBindingFree(bindings[i]);
+ VIR_FREE(bindings);
return ret;
error:
- for (; i < nbindings; i++)
- virNWFilterBindingFree(bindings[i]);
- VIR_FREE(bindings);
for (i = 0; i < nbindings; i++)
VIR_FREE(tmp[i]);
VIR_FREE(tmp);
@@ -645,20 +619,17 @@ virshSecretUUIDCompleter(vshControl *ctl,
if (virSecretGetUUIDString(secrets[i], uuid) < 0 ||
VIR_STRDUP(tmp[i], uuid) < 0)
goto error;
-
- virSecretFree(secrets[i]);
}
- VIR_FREE(secrets);
VIR_STEAL_PTR(ret, tmp);
cleanup:
+ for (i = 0; i < nsecrets; i++)
+ virSecretFree(secrets[i]);
+ VIR_FREE(secrets);
return ret;
error:
- for (; i < nsecrets; i++)
- virSecretFree(secrets[i]);
- VIR_FREE(secrets);
for (i = 0; i < nsecrets; i++)
VIR_FREE(tmp[i]);
VIR_FREE(tmp);
@@ -700,25 +671,21 @@ virshSnapshotNameCompleter(vshControl *ctl,
if (VIR_STRDUP(tmp[i], name) < 0)
goto error;
-
- virshDomainSnapshotFree(snapshots[i]);
}
- VIR_FREE(snapshots);
- virshDomainFree(dom);
VIR_STEAL_PTR(ret, tmp);
cleanup:
+ virshDomainFree(dom);
+ for (i = 0; i < nsnapshots; i++)
+ virshDomainSnapshotFree(snapshots[i]);
+ VIR_FREE(snapshots);
return ret;
error:
- for (; i < nsnapshots; i++)
- virshDomainSnapshotFree(snapshots[i]);
- VIR_FREE(snapshots);
for (i = 0; i < nsnapshots; i++)
VIR_FREE(tmp[i]);
VIR_FREE(tmp);
- virshDomainFree(dom);
goto cleanup;
}
--
2.20.1
3:48 p.m.
New subject: [libvirt] [PATCH 06/14] virsh-completer: unify cleanup of items in name completers
On Mon, Apr 01, 2019 at 09:33:23 +0200, Ján Tomko wrote:
Merge the cleanup of fetched items for the success and the error
paths.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
I'd probably squash this with the previous commit but I don't mind them
being separate. ACK to both.
3:33 p.m.
New subject: [libvirt] [PATCH 07/14] virsh-completer: virStringListFree where possible
We've been open-coding virStringListFreeCount for cleaning up
the completion list we're building. This had the advantage of
zeoring the pointer afterwards, which is no longer needed
now that we compile the list in 'tmp' instead of 'ret'.
Since all our lists are NULL-terminated anyway, switch to using
virStringListFree which has the benefit of being usable with
our VIR_AUTOSTRINGLIST macro.
Fixes nearly impossible NULL dereferences in
virshNWFilterBindingNameCompleter
virshNWFilterNameCompleter
virshNodeDeviceNameCompleter
virshNetworkNameCompleter
virshInterfaceNameCompleter
virshStoragePoolNameCompleter
virshDomainNameCompleter
which jumped on the error label after a failed allocation
and a possible one in
virshStorageVolNameCompleter
which jumped there when we fail to fetch the list of volumes.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
tools/virsh-completer.c | 67 +++++++++++------------------------------
1 file changed, 18 insertions(+), 49 deletions(-)
diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c
index 9d56659259..20b325c020 100644
--- a/tools/virsh-completer.c
+++ b/tools/virsh-completer.c
@@ -112,12 +112,10 @@ virshDomainNameCompleter(vshControl *ctl,
for (i = 0; i < ndomains; i++)
virshDomainFree(domains[i]);
VIR_FREE(domains);
+ virStringListFree(tmp);
return ret;
error:
- for (i = 0; i < ndomains; i++)
- VIR_FREE(tmp[i]);
- VIR_FREE(tmp);
goto cleanup;
}
@@ -262,12 +260,10 @@ virshStoragePoolNameCompleter(vshControl *ctl,
for (i = 0; i < npools; i++)
virStoragePoolFree(pools[i]);
VIR_FREE(pools);
+ virStringListFree(tmp);
return ret;
error:
- for (i = 0; i < npools; i++)
- VIR_FREE(tmp[i]);
- VIR_FREE(tmp);
goto cleanup;
}
@@ -315,12 +311,10 @@ virshStorageVolNameCompleter(vshControl *ctl,
for (i = 0; i < nvols; i++)
virStorageVolFree(vols[i]);
VIR_FREE(vols);
+ virStringListFree(tmp);
return ret;
error:
- for (i = 0; i < nvols; i++)
- VIR_FREE(tmp[i]);
- VIR_FREE(tmp);
goto cleanup;
}
@@ -363,12 +357,10 @@ virshInterfaceNameCompleter(vshControl *ctl,
for (i = 0; i < nifaces; i++)
virInterfaceFree(ifaces[i]);
VIR_FREE(ifaces);
+ virStringListFree(tmp);
return ret;
error:
- for (i = 0; i < nifaces; i++)
- VIR_FREE(tmp[i]);
- VIR_FREE(tmp);
goto cleanup;
}
@@ -412,12 +404,10 @@ virshNetworkNameCompleter(vshControl *ctl,
for (i = 0; i < nnets; i++)
virNetworkFree(nets[i]);
VIR_FREE(nets);
+ virStringListFree(tmp);
return ret;
error:
- for (i = 0; i < nnets; i++)
- VIR_FREE(tmp[i]);
- VIR_FREE(tmp);
goto cleanup;
}
@@ -444,10 +434,10 @@ virshNetworkEventNameCompleter(vshControl *ctl ATTRIBUTE_UNUSED,
VIR_STEAL_PTR(ret, tmp);
cleanup:
+ virStringListFree(tmp);
return ret;
error:
- virStringListFree(tmp);
goto cleanup;
}
@@ -488,12 +478,10 @@ virshNodeDeviceNameCompleter(vshControl *ctl,
for (i = 0; i < ndevs; i++)
virNodeDeviceFree(devs[i]);
VIR_FREE(devs);
+ virStringListFree(tmp);
return ret;
error:
- for (i = 0; i < ndevs; i++)
- VIR_FREE(tmp[i]);
- VIR_FREE(tmp);
goto cleanup;
}
@@ -534,12 +522,10 @@ virshNWFilterNameCompleter(vshControl *ctl,
for (i = 0; i < nnwfilters; i++)
virNWFilterFree(nwfilters[i]);
VIR_FREE(nwfilters);
+ virStringListFree(tmp);
return ret;
error:
- for (i = 0; i < nnwfilters; i++)
- VIR_FREE(tmp[i]);
- VIR_FREE(tmp);
goto cleanup;
}
@@ -580,12 +566,10 @@ virshNWFilterBindingNameCompleter(vshControl *ctl,
for (i = 0; i < nbindings; i++)
virNWFilterBindingFree(bindings[i]);
VIR_FREE(bindings);
+ virStringListFree(tmp);
return ret;
error:
- for (i = 0; i < nbindings; i++)
- VIR_FREE(tmp[i]);
- VIR_FREE(tmp);
goto cleanup;
}
@@ -627,12 +611,10 @@ virshSecretUUIDCompleter(vshControl *ctl,
for (i = 0; i < nsecrets; i++)
virSecretFree(secrets[i]);
VIR_FREE(secrets);
+ virStringListFree(tmp);
return ret;
error:
- for (i = 0; i < nsecrets; i++)
- VIR_FREE(tmp[i]);
- VIR_FREE(tmp);
goto cleanup;
}
@@ -680,12 +662,10 @@ virshSnapshotNameCompleter(vshControl *ctl,
for (i = 0; i < nsnapshots; i++)
virshDomainSnapshotFree(snapshots[i]);
VIR_FREE(snapshots);
+ virStringListFree(tmp);
return ret;
error:
- for (i = 0; i < nsnapshots; i++)
- VIR_FREE(tmp[i]);
- VIR_FREE(tmp);
goto cleanup;
}
@@ -764,15 +744,10 @@ virshAllocpagesPagesizeCompleter(vshControl *ctl,
VIR_FREE(pagesize);
VIR_FREE(cap_xml);
VIR_FREE(unit);
-
+ virStringListFree(tmp);
return ret;
error:
- if (tmp) {
- for (i = 0; i < npages; i++)
- VIR_FREE(tmp[i]);
- }
- VIR_FREE(tmp);
goto cleanup;
}
@@ -799,10 +774,10 @@ virshSecretEventNameCompleter(vshControl *ctl ATTRIBUTE_UNUSED,
VIR_STEAL_PTR(ret, tmp);
cleanup:
+ virStringListFree(tmp);
return ret;
error:
- virStringListFree(tmp);
goto cleanup;
}
@@ -829,10 +804,10 @@ virshDomainEventNameCompleter(vshControl *ctl ATTRIBUTE_UNUSED,
VIR_STEAL_PTR(ret, tmp);
cleanup:
+ virStringListFree(tmp);
return ret;
error:
- virStringListFree(tmp);
goto cleanup;
}
@@ -859,10 +834,10 @@ virshPoolEventNameCompleter(vshControl *ctl ATTRIBUTE_UNUSED,
VIR_STEAL_PTR(ret, tmp);
cleanup:
+ virStringListFree(tmp);
return ret;
error:
- virStringListFree(tmp);
goto cleanup;
}
@@ -933,11 +908,10 @@ virshDomainInterfaceStateCompleter(vshControl *ctl,
VIR_FREE(interfaces);
xmlXPathFreeContext(ctxt);
xmlFreeDoc(xml);
+ virStringListFree(tmp);
return ret;
error:
- virStringListFree(tmp);
- tmp = NULL;
goto cleanup;
}
@@ -964,10 +938,10 @@ virshNodedevEventNameCompleter(vshControl *ctl ATTRIBUTE_UNUSED,
VIR_STEAL_PTR(ret, tmp);
cleanup:
+ virStringListFree(tmp);
return ret;
error:
- virStringListFree(tmp);
goto cleanup;
}
@@ -1017,15 +991,10 @@ virshCellnoCompleter(vshControl *ctl,
VIR_FREE(cells);
xmlFreeDoc(doc);
VIR_FREE(cap_xml);
-
+ virStringListFree(tmp);
return ret;
error:
- if (tmp) {
- for (i = 0; i < ncells; i++)
- VIR_FREE(tmp[i]);
- }
- VIR_FREE(tmp);
goto cleanup;
}
--
2.20.1
3:54 p.m.
New subject: [libvirt] [PATCH 07/14] virsh-completer: virStringListFree where possible
On Mon, Apr 01, 2019 at 09:33:24 +0200, Ján Tomko wrote:
We've been open-coding virStringListFreeCount for cleaning up
the completion list we're building. This had the advantage of
zeoring the pointer afterwards, which is no longer needed
now that we compile the list in 'tmp' instead of 'ret'.
But now you are not preserving the holy NULLs on the stack!!.
Since all our lists are NULL-terminated anyway, switch to using
virStringListFree which has the benefit of being usable with
our VIR_AUTOSTRINGLIST macro.
Fixes nearly impossible NULL dereferences in
virshNWFilterBindingNameCompleter
virshNWFilterNameCompleter
virshNodeDeviceNameCompleter
virshNetworkNameCompleter
virshInterfaceNameCompleter
virshStoragePoolNameCompleter
virshDomainNameCompleter
which jumped on the error label after a failed allocation
and a possible one in
virshStorageVolNameCompleter
which jumped there when we fail to fetch the list of volumes.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
ACK
3:33 p.m.
New subject: [libvirt] [PATCH 08/14] virsh-completer: use VIR_AUTOSTRINGLIST for tmp
Now that every function uses virStringListFree, we can use the
VIR_AUTOSTRINGLIST to have the compiler call this function automatically
when 'tmp' goes out of scope.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
tools/virsh-completer.c | 63 ++++++++++++++---------------------------
1 file changed, 21 insertions(+), 42 deletions(-)
diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c
index 20b325c020..669eda0f84 100644
--- a/tools/virsh-completer.c
+++ b/tools/virsh-completer.c
@@ -79,7 +79,7 @@ virshDomainNameCompleter(vshControl *ctl,
int ndomains = 0;
size_t i = 0;
char **ret = NULL;
- char **tmp = NULL;
+ VIR_AUTOSTRINGLIST tmp = NULL;
virCheckFlags(VIR_CONNECT_LIST_DOMAINS_ACTIVE |
VIR_CONNECT_LIST_DOMAINS_INACTIVE |
@@ -112,7 +112,6 @@ virshDomainNameCompleter(vshControl *ctl,
for (i = 0; i < ndomains; i++)
virshDomainFree(domains[i]);
VIR_FREE(domains);
- virStringListFree(tmp);
return ret;
error:
@@ -133,7 +132,7 @@ virshDomainInterfaceCompleter(vshControl *ctl,
size_t i;
unsigned int domainXMLFlags = 0;
char **ret = NULL;
- char **tmp = NULL;
+ VIR_AUTOSTRINGLIST tmp = NULL;
virCheckFlags(VIRSH_DOMAIN_INTERFACE_COMPLETER_MAC, NULL);
@@ -171,7 +170,6 @@ virshDomainInterfaceCompleter(vshControl *ctl,
VIR_FREE(interfaces);
xmlFreeDoc(xmldoc);
xmlXPathFreeContext(ctxt);
- virStringListFree(tmp);
return ret;
}
@@ -187,7 +185,7 @@ virshDomainDiskTargetCompleter(vshControl *ctl,
xmlNodePtr *disks = NULL;
int ndisks;
size_t i;
- char **tmp = NULL;
+ VIR_AUTOSTRINGLIST tmp = NULL;
char **ret = NULL;
virCheckFlags(0, NULL);
@@ -216,7 +214,6 @@ virshDomainDiskTargetCompleter(vshControl *ctl,
VIR_FREE(disks);
xmlFreeDoc(xmldoc);
xmlXPathFreeContext(ctxt);
- virStringListFree(tmp);
return ret;
}
@@ -231,7 +228,7 @@ virshStoragePoolNameCompleter(vshControl *ctl,
int npools = 0;
size_t i = 0;
char **ret = NULL;
- char **tmp = NULL;
+ VIR_AUTOSTRINGLIST tmp = NULL;
virCheckFlags(VIR_CONNECT_LIST_STORAGE_POOLS_INACTIVE |
VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE |
@@ -260,7 +257,6 @@ virshStoragePoolNameCompleter(vshControl *ctl,
for (i = 0; i < npools; i++)
virStoragePoolFree(pools[i]);
VIR_FREE(pools);
- virStringListFree(tmp);
return ret;
error:
@@ -280,7 +276,7 @@ virshStorageVolNameCompleter(vshControl *ctl,
int nvols = 0;
size_t i = 0;
char **ret = NULL;
- char **tmp = NULL;
+ VIR_AUTOSTRINGLIST tmp = NULL;
virCheckFlags(0, NULL);
@@ -311,7 +307,6 @@ virshStorageVolNameCompleter(vshControl *ctl,
for (i = 0; i < nvols; i++)
virStorageVolFree(vols[i]);
VIR_FREE(vols);
- virStringListFree(tmp);
return ret;
error:
@@ -329,7 +324,7 @@ virshInterfaceNameCompleter(vshControl *ctl,
int nifaces = 0;
size_t i = 0;
char **ret = NULL;
- char **tmp = NULL;
+ VIR_AUTOSTRINGLIST tmp = NULL;
virCheckFlags(VIR_CONNECT_LIST_INTERFACES_ACTIVE |
VIR_CONNECT_LIST_INTERFACES_INACTIVE,
@@ -357,7 +352,6 @@ virshInterfaceNameCompleter(vshControl *ctl,
for (i = 0; i < nifaces; i++)
virInterfaceFree(ifaces[i]);
VIR_FREE(ifaces);
- virStringListFree(tmp);
return ret;
error:
@@ -375,7 +369,7 @@ virshNetworkNameCompleter(vshControl *ctl,
int nnets = 0;
size_t i = 0;
char **ret = NULL;
- char **tmp = NULL;
+ VIR_AUTOSTRINGLIST tmp = NULL;
virCheckFlags(VIR_CONNECT_LIST_NETWORKS_INACTIVE |
VIR_CONNECT_LIST_NETWORKS_ACTIVE |
@@ -404,7 +398,6 @@ virshNetworkNameCompleter(vshControl *ctl,
for (i = 0; i < nnets; i++)
virNetworkFree(nets[i]);
VIR_FREE(nets);
- virStringListFree(tmp);
return ret;
error:
@@ -419,7 +412,7 @@ virshNetworkEventNameCompleter(vshControl *ctl ATTRIBUTE_UNUSED,
{
size_t i = 0;
char **ret = NULL;
- char **tmp = NULL;
+ VIR_AUTOSTRINGLIST tmp = NULL;
virCheckFlags(0, NULL);
@@ -434,7 +427,6 @@ virshNetworkEventNameCompleter(vshControl *ctl ATTRIBUTE_UNUSED,
VIR_STEAL_PTR(ret, tmp);
cleanup:
- virStringListFree(tmp);
return ret;
error:
@@ -452,7 +444,7 @@ virshNodeDeviceNameCompleter(vshControl *ctl,
int ndevs = 0;
size_t i = 0;
char **ret = NULL;
- char **tmp = NULL;
+ VIR_AUTOSTRINGLIST tmp = NULL;
virCheckFlags(0, NULL);
@@ -478,7 +470,6 @@ virshNodeDeviceNameCompleter(vshControl *ctl,
for (i = 0; i < ndevs; i++)
virNodeDeviceFree(devs[i]);
VIR_FREE(devs);
- virStringListFree(tmp);
return ret;
error:
@@ -496,7 +487,7 @@ virshNWFilterNameCompleter(vshControl *ctl,
int nnwfilters = 0;
size_t i = 0;
char **ret = NULL;
- char **tmp = NULL;
+ VIR_AUTOSTRINGLIST tmp = NULL;
virCheckFlags(0, NULL);
@@ -522,7 +513,6 @@ virshNWFilterNameCompleter(vshControl *ctl,
for (i = 0; i < nnwfilters; i++)
virNWFilterFree(nwfilters[i]);
VIR_FREE(nwfilters);
- virStringListFree(tmp);
return ret;
error:
@@ -540,7 +530,7 @@ virshNWFilterBindingNameCompleter(vshControl *ctl,
int nbindings = 0;
size_t i = 0;
char **ret = NULL;
- char **tmp = NULL;
+ VIR_AUTOSTRINGLIST tmp = NULL;
virCheckFlags(0, NULL);
@@ -566,7 +556,6 @@ virshNWFilterBindingNameCompleter(vshControl *ctl,
for (i = 0; i < nbindings; i++)
virNWFilterBindingFree(bindings[i]);
VIR_FREE(bindings);
- virStringListFree(tmp);
return ret;
error:
@@ -584,7 +573,7 @@ virshSecretUUIDCompleter(vshControl *ctl,
int nsecrets = 0;
size_t i = 0;
char **ret = NULL;
- char **tmp = NULL;
+ VIR_AUTOSTRINGLIST tmp = NULL;
virCheckFlags(0, NULL);
@@ -611,7 +600,6 @@ virshSecretUUIDCompleter(vshControl *ctl,
for (i = 0; i < nsecrets; i++)
virSecretFree(secrets[i]);
VIR_FREE(secrets);
- virStringListFree(tmp);
return ret;
error:
@@ -631,7 +619,7 @@ virshSnapshotNameCompleter(vshControl *ctl,
int nsnapshots = 0;
size_t i = 0;
char **ret = NULL;
- char **tmp = NULL;
+ VIR_AUTOSTRINGLIST tmp = NULL;
virCheckFlags(0, NULL);
@@ -662,7 +650,6 @@ virshSnapshotNameCompleter(vshControl *ctl,
for (i = 0; i < nsnapshots; i++)
virshDomainSnapshotFree(snapshots[i]);
VIR_FREE(snapshots);
- virStringListFree(tmp);
return ret;
error:
@@ -690,7 +677,7 @@ virshAllocpagesPagesizeCompleter(vshControl *ctl,
char *cap_xml = NULL;
char **ret = NULL;
char *unit = NULL;
- char **tmp = NULL;
+ VIR_AUTOSTRINGLIST tmp = NULL;
virCheckFlags(0, NULL);
@@ -744,7 +731,6 @@ virshAllocpagesPagesizeCompleter(vshControl *ctl,
VIR_FREE(pagesize);
VIR_FREE(cap_xml);
VIR_FREE(unit);
- virStringListFree(tmp);
return ret;
error:
@@ -759,7 +745,7 @@ virshSecretEventNameCompleter(vshControl *ctl ATTRIBUTE_UNUSED,
{
size_t i;
char **ret = NULL;
- char **tmp = NULL;
+ VIR_AUTOSTRINGLIST tmp = NULL;
virCheckFlags(0, NULL);
@@ -774,7 +760,6 @@ virshSecretEventNameCompleter(vshControl *ctl ATTRIBUTE_UNUSED,
VIR_STEAL_PTR(ret, tmp);
cleanup:
- virStringListFree(tmp);
return ret;
error:
@@ -789,7 +774,7 @@ virshDomainEventNameCompleter(vshControl *ctl ATTRIBUTE_UNUSED,
{
size_t i = 0;
char **ret = NULL;
- char **tmp = NULL;
+ VIR_AUTOSTRINGLIST tmp = NULL;
virCheckFlags(0, NULL);
@@ -804,7 +789,6 @@ virshDomainEventNameCompleter(vshControl *ctl ATTRIBUTE_UNUSED,
VIR_STEAL_PTR(ret, tmp);
cleanup:
- virStringListFree(tmp);
return ret;
error:
@@ -819,7 +803,7 @@ virshPoolEventNameCompleter(vshControl *ctl ATTRIBUTE_UNUSED,
{
size_t i = 0;
char **ret = NULL;
- char **tmp = NULL;
+ VIR_AUTOSTRINGLIST tmp = NULL;
virCheckFlags(0, NULL);
@@ -834,7 +818,6 @@ virshPoolEventNameCompleter(vshControl *ctl ATTRIBUTE_UNUSED,
VIR_STEAL_PTR(ret, tmp);
cleanup:
- virStringListFree(tmp);
return ret;
error:
@@ -858,7 +841,7 @@ virshDomainInterfaceStateCompleter(vshControl *ctl,
xmlNodePtr *interfaces = NULL;
char *xpath = NULL;
char *state = NULL;
- char **tmp = NULL;
+ VIR_AUTOSTRINGLIST tmp = NULL;
virCheckFlags(0, NULL);
@@ -908,7 +891,6 @@ virshDomainInterfaceStateCompleter(vshControl *ctl,
VIR_FREE(interfaces);
xmlXPathFreeContext(ctxt);
xmlFreeDoc(xml);
- virStringListFree(tmp);
return ret;
error:
@@ -923,7 +905,7 @@ virshNodedevEventNameCompleter(vshControl *ctl ATTRIBUTE_UNUSED,
{
size_t i = 0;
char **ret = NULL;
- char **tmp = NULL;
+ VIR_AUTOSTRINGLIST tmp = NULL;
virCheckFlags(0, NULL);
@@ -938,7 +920,6 @@ virshNodedevEventNameCompleter(vshControl *ctl ATTRIBUTE_UNUSED,
VIR_STEAL_PTR(ret, tmp);
cleanup:
- virStringListFree(tmp);
return ret;
error:
@@ -959,7 +940,7 @@ virshCellnoCompleter(vshControl *ctl,
size_t i = 0;
char *cap_xml = NULL;
char **ret = NULL;
- char **tmp = NULL;
+ VIR_AUTOSTRINGLIST tmp = NULL;
virCheckFlags(0, NULL);
@@ -991,7 +972,6 @@ virshCellnoCompleter(vshControl *ctl,
VIR_FREE(cells);
xmlFreeDoc(doc);
VIR_FREE(cap_xml);
- virStringListFree(tmp);
return ret;
error:
@@ -1012,7 +992,7 @@ virshDomainDeviceAliasCompleter(vshControl *ctl,
size_t i;
unsigned int domainXMLFlags = 0;
char **ret = NULL;
- char **tmp = NULL;
+ VIR_AUTOSTRINGLIST tmp = NULL;
virCheckFlags(0, NULL);
@@ -1042,6 +1022,5 @@ virshDomainDeviceAliasCompleter(vshControl *ctl,
VIR_FREE(aliases);
xmlFreeDoc(xmldoc);
xmlXPathFreeContext(ctxt);
- virStringListFree(tmp);
return ret;
}
--
2.20.1
3:55 p.m.
New subject: [libvirt] [PATCH 08/14] virsh-completer: use VIR_AUTOSTRINGLIST for tmp
On Mon, Apr 01, 2019 at 09:33:25 +0200, Ján Tomko wrote:
Now that every function uses virStringListFree, we can use the
VIR_AUTOSTRINGLIST to have the compiler call this function automatically
when 'tmp' goes out of scope.
Again something that could be done in previous patch directly. You are
adding a bunch of calls to virStringListFree which vanish here.
ACK but really consider squashing this one.
4:36 p.m.
New subject: [libvirt] [PATCH 08/14] virsh-completer: use VIR_AUTOSTRINGLIST for tmp
On Wed, Apr 03, 2019 at 09:55:33AM +0200, Peter Krempa wrote:
On Mon, Apr 01, 2019 at 09:33:25 +0200, Ján Tomko wrote:
> Now that every function uses virStringListFree, we can use the
> VIR_AUTOSTRINGLIST to have the compiler call this function automatically
> when 'tmp' goes out of scope.
Again something that could be done in previous patch directly. You are
adding a bunch of calls to virStringListFree which vanish here.
ACK but really consider squashing this one.
Squashed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
3:33 p.m.
New subject: [libvirt] [PATCH 09/14] use VIR_AUTOFREE for xmlNodePtr* variables
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
tools/virsh-completer.c | 18 ++++++------------
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c
index 669eda0f84..3d107f0504 100644
--- a/tools/virsh-completer.c
+++ b/tools/virsh-completer.c
@@ -128,7 +128,7 @@ virshDomainInterfaceCompleter(vshControl *ctl,
xmlDocPtr xmldoc = NULL;
xmlXPathContextPtr ctxt = NULL;
int ninterfaces;
- xmlNodePtr *interfaces = NULL;
+ VIR_AUTOFREE(xmlNodePtr *) interfaces = NULL;
size_t i;
unsigned int domainXMLFlags = 0;
char **ret = NULL;
@@ -167,7 +167,6 @@ virshDomainInterfaceCompleter(vshControl *ctl,
VIR_STEAL_PTR(ret, tmp);
cleanup:
- VIR_FREE(interfaces);
xmlFreeDoc(xmldoc);
xmlXPathFreeContext(ctxt);
return ret;
@@ -182,7 +181,7 @@ virshDomainDiskTargetCompleter(vshControl *ctl,
virshControlPtr priv = ctl->privData;
xmlDocPtr xmldoc = NULL;
xmlXPathContextPtr ctxt = NULL;
- xmlNodePtr *disks = NULL;
+ VIR_AUTOFREE(xmlNodePtr *) disks = NULL;
int ndisks;
size_t i;
VIR_AUTOSTRINGLIST tmp = NULL;
@@ -211,7 +210,6 @@ virshDomainDiskTargetCompleter(vshControl *ctl,
VIR_STEAL_PTR(ret, tmp);
cleanup:
- VIR_FREE(disks);
xmlFreeDoc(xmldoc);
xmlXPathFreeContext(ctxt);
return ret;
@@ -665,7 +663,7 @@ virshAllocpagesPagesizeCompleter(vshControl *ctl,
xmlXPathContextPtr ctxt = NULL;
virshControlPtr priv = ctl->privData;
unsigned int npages = 0;
- xmlNodePtr *pages = NULL;
+ VIR_AUTOFREE(xmlNodePtr *) pages = NULL;
xmlDocPtr doc = NULL;
double size = 0;
size_t i = 0;
@@ -725,7 +723,6 @@ virshAllocpagesPagesizeCompleter(vshControl *ctl,
cleanup:
xmlXPathFreeContext(ctxt);
- VIR_FREE(pages);
xmlFreeDoc(doc);
VIR_FREE(path);
VIR_FREE(pagesize);
@@ -838,7 +835,7 @@ virshDomainInterfaceStateCompleter(vshControl *ctl,
virMacAddr macaddr;
char macstr[VIR_MAC_STRING_BUFLEN] = "";
int ninterfaces;
- xmlNodePtr *interfaces = NULL;
+ VIR_AUTOFREE(xmlNodePtr *) interfaces = NULL;
char *xpath = NULL;
char *state = NULL;
VIR_AUTOSTRINGLIST tmp = NULL;
@@ -888,7 +885,6 @@ virshDomainInterfaceStateCompleter(vshControl *ctl,
cleanup:
VIR_FREE(state);
VIR_FREE(xpath);
- VIR_FREE(interfaces);
xmlXPathFreeContext(ctxt);
xmlFreeDoc(xml);
return ret;
@@ -935,7 +931,7 @@ virshCellnoCompleter(vshControl *ctl,
xmlXPathContextPtr ctxt = NULL;
virshControlPtr priv = ctl->privData;
unsigned int ncells = 0;
- xmlNodePtr *cells = NULL;
+ VIR_AUTOFREE(xmlNodePtr *) cells = NULL;
xmlDocPtr doc = NULL;
size_t i = 0;
char *cap_xml = NULL;
@@ -969,7 +965,6 @@ virshCellnoCompleter(vshControl *ctl,
cleanup:
xmlXPathFreeContext(ctxt);
- VIR_FREE(cells);
xmlFreeDoc(doc);
VIR_FREE(cap_xml);
return ret;
@@ -988,7 +983,7 @@ virshDomainDeviceAliasCompleter(vshControl *ctl,
xmlDocPtr xmldoc = NULL;
xmlXPathContextPtr ctxt = NULL;
int naliases;
- xmlNodePtr *aliases = NULL;
+ VIR_AUTOFREE(xmlNodePtr *) aliases = NULL;
size_t i;
unsigned int domainXMLFlags = 0;
char **ret = NULL;
@@ -1019,7 +1014,6 @@ virshDomainDeviceAliasCompleter(vshControl *ctl,
VIR_STEAL_PTR(ret, tmp);
cleanup:
- VIR_FREE(aliases);
xmlFreeDoc(xmldoc);
xmlXPathFreeContext(ctxt);
return ret;
--
2.20.1
3:56 p.m.
New subject: [libvirt] [PATCH 09/14] use VIR_AUTOFREE for xmlNodePtr* variables
On Mon, Apr 01, 2019 at 09:33:26 +0200, Ján Tomko wrote:
Add 'virsh-completer:' into the subject.
ACK
3:33 p.m.
New subject: [libvirt] [PATCH 10/14] virsh-completer: use VIR_AUTOPTR for xml* variables
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
tools/virsh-completer.c | 36 ++++++++++++------------------------
1 file changed, 12 insertions(+), 24 deletions(-)
diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c
index 3d107f0504..dc4fe2b83a 100644
--- a/tools/virsh-completer.c
+++ b/tools/virsh-completer.c
@@ -125,8 +125,8 @@ virshDomainInterfaceCompleter(vshControl *ctl,
unsigned int flags)
{
virshControlPtr priv = ctl->privData;
- xmlDocPtr xmldoc = NULL;
- xmlXPathContextPtr ctxt = NULL;
+ VIR_AUTOPTR(xmlDoc) xmldoc = NULL;
+ VIR_AUTOPTR(xmlXPathContext) ctxt = NULL;
int ninterfaces;
VIR_AUTOFREE(xmlNodePtr *) interfaces = NULL;
size_t i;
@@ -167,8 +167,6 @@ virshDomainInterfaceCompleter(vshControl *ctl,
VIR_STEAL_PTR(ret, tmp);
cleanup:
- xmlFreeDoc(xmldoc);
- xmlXPathFreeContext(ctxt);
return ret;
}
@@ -179,8 +177,8 @@ virshDomainDiskTargetCompleter(vshControl *ctl,
unsigned int flags)
{
virshControlPtr priv = ctl->privData;
- xmlDocPtr xmldoc = NULL;
- xmlXPathContextPtr ctxt = NULL;
+ VIR_AUTOPTR(xmlDoc) xmldoc = NULL;
+ VIR_AUTOPTR(xmlXPathContext) ctxt = NULL;
VIR_AUTOFREE(xmlNodePtr *) disks = NULL;
int ndisks;
size_t i;
@@ -210,8 +208,6 @@ virshDomainDiskTargetCompleter(vshControl *ctl,
VIR_STEAL_PTR(ret, tmp);
cleanup:
- xmlFreeDoc(xmldoc);
- xmlXPathFreeContext(ctxt);
return ret;
}
@@ -660,11 +656,11 @@ virshAllocpagesPagesizeCompleter(vshControl *ctl,
unsigned int flags)
{
unsigned long long byteval = 0;
- xmlXPathContextPtr ctxt = NULL;
+ VIR_AUTOPTR(xmlXPathContext) ctxt = NULL;
virshControlPtr priv = ctl->privData;
unsigned int npages = 0;
VIR_AUTOFREE(xmlNodePtr *) pages = NULL;
- xmlDocPtr doc = NULL;
+ VIR_AUTOPTR(xmlDoc) doc = NULL;
double size = 0;
size_t i = 0;
const char *suffix = NULL;
@@ -722,8 +718,6 @@ virshAllocpagesPagesizeCompleter(vshControl *ctl,
VIR_STEAL_PTR(ret, tmp);
cleanup:
- xmlXPathFreeContext(ctxt);
- xmlFreeDoc(doc);
VIR_FREE(path);
VIR_FREE(pagesize);
VIR_FREE(cap_xml);
@@ -830,8 +824,8 @@ virshDomainInterfaceStateCompleter(vshControl *ctl,
virshControlPtr priv = ctl->privData;
const char *iface = NULL;
char **ret = NULL;
- xmlDocPtr xml = NULL;
- xmlXPathContextPtr ctxt = NULL;
+ VIR_AUTOPTR(xmlDoc) xml = NULL;
+ VIR_AUTOPTR(xmlXPathContext) ctxt = NULL;
virMacAddr macaddr;
char macstr[VIR_MAC_STRING_BUFLEN] = "";
int ninterfaces;
@@ -885,8 +879,6 @@ virshDomainInterfaceStateCompleter(vshControl *ctl,
cleanup:
VIR_FREE(state);
VIR_FREE(xpath);
- xmlXPathFreeContext(ctxt);
- xmlFreeDoc(xml);
return ret;
error:
@@ -928,11 +920,11 @@ virshCellnoCompleter(vshControl *ctl,
const vshCmd *cmd ATTRIBUTE_UNUSED,
unsigned int flags)
{
- xmlXPathContextPtr ctxt = NULL;
+ VIR_AUTOPTR(xmlXPathContext) ctxt = NULL;
virshControlPtr priv = ctl->privData;
unsigned int ncells = 0;
VIR_AUTOFREE(xmlNodePtr *) cells = NULL;
- xmlDocPtr doc = NULL;
+ VIR_AUTOPTR(xmlDoc) doc = NULL;
size_t i = 0;
char *cap_xml = NULL;
char **ret = NULL;
@@ -964,8 +956,6 @@ virshCellnoCompleter(vshControl *ctl,
VIR_STEAL_PTR(ret, tmp);
cleanup:
- xmlXPathFreeContext(ctxt);
- xmlFreeDoc(doc);
VIR_FREE(cap_xml);
return ret;
@@ -980,8 +970,8 @@ virshDomainDeviceAliasCompleter(vshControl *ctl,
unsigned int flags)
{
virshControlPtr priv = ctl->privData;
- xmlDocPtr xmldoc = NULL;
- xmlXPathContextPtr ctxt = NULL;
+ VIR_AUTOPTR(xmlDoc) xmldoc = NULL;
+ VIR_AUTOPTR(xmlXPathContext) ctxt = NULL;
int naliases;
VIR_AUTOFREE(xmlNodePtr *) aliases = NULL;
size_t i;
@@ -1014,7 +1004,5 @@ virshDomainDeviceAliasCompleter(vshControl *ctl,
VIR_STEAL_PTR(ret, tmp);
cleanup:
- xmlFreeDoc(xmldoc);
- xmlXPathFreeContext(ctxt);
return ret;
}
--
2.20.1
3:58 p.m.
New subject: [libvirt] [PATCH 10/14] virsh-completer: use VIR_AUTOPTR for xml* variables
On Mon, Apr 01, 2019 at 09:33:27 +0200, Ján Tomko wrote:
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
tools/virsh-completer.c | 36 ++++++++++++------------------------
1 file changed, 12 insertions(+), 24 deletions(-)
ACK
3:33 p.m.
New subject: [libvirt] [PATCH 11/14] virsh-completer: use VIR_AUTOFREE for char* variables
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
tools/virsh-completer.c | 21 +++++++--------------
1 file changed, 7 insertions(+), 14 deletions(-)
diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c
index dc4fe2b83a..025eee19e2 100644
--- a/tools/virsh-completer.c
+++ b/tools/virsh-completer.c
@@ -666,11 +666,11 @@ virshAllocpagesPagesizeCompleter(vshControl *ctl,
const char *suffix = NULL;
const char *cellnum = NULL;
bool cellno = vshCommandOptBool(cmd, "cellno");
- char *path = NULL;
- char *pagesize = NULL;
- char *cap_xml = NULL;
+ VIR_AUTOFREE(char *) path = NULL;
+ VIR_AUTOFREE(char *) pagesize = NULL;
+ VIR_AUTOFREE(char *) cap_xml = NULL;
char **ret = NULL;
- char *unit = NULL;
+ VIR_AUTOFREE(char *) unit = NULL;
VIR_AUTOSTRINGLIST tmp = NULL;
virCheckFlags(0, NULL);
@@ -718,10 +718,6 @@ virshAllocpagesPagesizeCompleter(vshControl *ctl,
VIR_STEAL_PTR(ret, tmp);
cleanup:
- VIR_FREE(path);
- VIR_FREE(pagesize);
- VIR_FREE(cap_xml);
- VIR_FREE(unit);
return ret;
error:
@@ -830,8 +826,8 @@ virshDomainInterfaceStateCompleter(vshControl *ctl,
char macstr[VIR_MAC_STRING_BUFLEN] = "";
int ninterfaces;
VIR_AUTOFREE(xmlNodePtr *) interfaces = NULL;
- char *xpath = NULL;
- char *state = NULL;
+ VIR_AUTOFREE(char *) xpath = NULL;
+ VIR_AUTOFREE(char *) state = NULL;
VIR_AUTOSTRINGLIST tmp = NULL;
virCheckFlags(0, NULL);
@@ -877,8 +873,6 @@ virshDomainInterfaceStateCompleter(vshControl *ctl,
VIR_STEAL_PTR(ret, tmp);
cleanup:
- VIR_FREE(state);
- VIR_FREE(xpath);
return ret;
error:
@@ -926,7 +920,7 @@ virshCellnoCompleter(vshControl *ctl,
VIR_AUTOFREE(xmlNodePtr *) cells = NULL;
VIR_AUTOPTR(xmlDoc) doc = NULL;
size_t i = 0;
- char *cap_xml = NULL;
+ VIR_AUTOFREE(char *) cap_xml = NULL;
char **ret = NULL;
VIR_AUTOSTRINGLIST tmp = NULL;
@@ -956,7 +950,6 @@ virshCellnoCompleter(vshControl *ctl,
VIR_STEAL_PTR(ret, tmp);
cleanup:
- VIR_FREE(cap_xml);
return ret;
error:
--
2.20.1
3:59 p.m.
New subject: [libvirt] [PATCH 11/14] virsh-completer: use VIR_AUTOFREE for char* variables
On Mon, Apr 01, 2019 at 09:33:28 +0200, Ján Tomko wrote:
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
tools/virsh-completer.c | 21 +++++++--------------
1 file changed, 7 insertions(+), 14 deletions(-)
ACK
3:33 p.m.
New subject: [libvirt] [PATCH 12/14] virsh-completer: remove excessive labels
Now that we have a shared cleanup section everywhere,
delete all the 'error' labels which all contain just 'goto cleanup'
anyway.
Also remove all the 'cleanup' labels that only 'return ret' - we
can simply return NULL instead of jumping to that label.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
tools/virsh-completer.c | 207 +++++++++++++---------------------------
1 file changed, 68 insertions(+), 139 deletions(-)
diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c
index 025eee19e2..2b9749300a 100644
--- a/tools/virsh-completer.c
+++ b/tools/virsh-completer.c
@@ -97,13 +97,13 @@ virshDomainNameCompleter(vshControl *ctl,
return NULL;
if (VIR_ALLOC_N(tmp, ndomains + 1) < 0)
- goto error;
+ goto cleanup;
for (i = 0; i < ndomains; i++) {
const char *name = virDomainGetName(domains[i]);
if (VIR_STRDUP(tmp[i], name) < 0)
- goto error;
+ goto cleanup;
}
VIR_STEAL_PTR(ret, tmp);
@@ -113,9 +113,6 @@ virshDomainNameCompleter(vshControl *ctl,
virshDomainFree(domains[i]);
VIR_FREE(domains);
return ret;
-
- error:
- goto cleanup;
}
@@ -143,14 +140,14 @@ virshDomainInterfaceCompleter(vshControl *ctl,
domainXMLFlags = VIR_DOMAIN_XML_INACTIVE;
if (virshDomainGetXML(ctl, cmd, domainXMLFlags, &xmldoc, &ctxt) < 0)
- goto cleanup;
+ return NULL;
ninterfaces = virXPathNodeSet("./devices/interface", ctxt,
&interfaces);
if (ninterfaces < 0)
- goto cleanup;
+ return NULL;
if (VIR_ALLOC_N(tmp, ninterfaces + 1) < 0)
- goto cleanup;
+ return NULL;
for (i = 0; i < ninterfaces; i++) {
ctxt->node = interfaces[i];
@@ -162,11 +159,10 @@ virshDomainInterfaceCompleter(vshControl *ctl,
/* In case we are dealing with inactive domain XML there's no
* <target dev=''/>. Offer MAC addresses then. */
if (!(tmp[i] = virXPathString("string(./mac/@address)", ctxt)))
- goto cleanup;
+ return NULL;
}
VIR_STEAL_PTR(ret, tmp);
- cleanup:
return ret;
}
@@ -191,23 +187,22 @@ virshDomainDiskTargetCompleter(vshControl *ctl,
return NULL;
if (virshDomainGetXML(ctl, cmd, 0, &xmldoc, &ctxt) < 0)
- goto cleanup;
+ return NULL;
ndisks = virXPathNodeSet("./devices/disk", ctxt, &disks);
if (ndisks < 0)
- goto cleanup;
+ return NULL;
if (VIR_ALLOC_N(tmp, ndisks + 1) < 0)
- goto cleanup;
+ return NULL;
for (i = 0; i < ndisks; i++) {
ctxt->node = disks[i];
if (!(tmp[i] = virXPathString("string(./target/@dev)", ctxt)))
- goto cleanup;
+ return NULL;
}
VIR_STEAL_PTR(ret, tmp);
- cleanup:
return ret;
}
@@ -236,13 +231,13 @@ virshStoragePoolNameCompleter(vshControl *ctl,
return NULL;
if (VIR_ALLOC_N(tmp, npools + 1) < 0)
- goto error;
+ goto cleanup;
for (i = 0; i < npools; i++) {
const char *name = virStoragePoolGetName(pools[i]);
if (VIR_STRDUP(tmp[i], name) < 0)
- goto error;
+ goto cleanup;
}
VIR_STEAL_PTR(ret, tmp);
@@ -252,9 +247,6 @@ virshStoragePoolNameCompleter(vshControl *ctl,
virStoragePoolFree(pools[i]);
VIR_FREE(pools);
return ret;
-
- error:
- goto cleanup;
}
@@ -281,17 +273,17 @@ virshStorageVolNameCompleter(vshControl *ctl,
return NULL;
if ((rc = virStoragePoolListAllVolumes(pool, &vols, flags)) < 0)
- goto error;
+ goto cleanup;
nvols = rc;
if (VIR_ALLOC_N(tmp, nvols + 1) < 0)
- goto error;
+ goto cleanup;
for (i = 0; i < nvols; i++) {
const char *name = virStorageVolGetName(vols[i]);
if (VIR_STRDUP(tmp[i], name) < 0)
- goto error;
+ goto cleanup;
}
VIR_STEAL_PTR(ret, tmp);
@@ -302,9 +294,6 @@ virshStorageVolNameCompleter(vshControl *ctl,
virStorageVolFree(vols[i]);
VIR_FREE(vols);
return ret;
-
- error:
- goto cleanup;
}
@@ -331,13 +320,13 @@ virshInterfaceNameCompleter(vshControl *ctl,
return NULL;
if (VIR_ALLOC_N(tmp, nifaces + 1) < 0)
- goto error;
+ goto cleanup;
for (i = 0; i < nifaces; i++) {
const char *name = virInterfaceGetName(ifaces[i]);
if (VIR_STRDUP(tmp[i], name) < 0)
- goto error;
+ goto cleanup;
}
VIR_STEAL_PTR(ret, tmp);
@@ -347,9 +336,6 @@ virshInterfaceNameCompleter(vshControl *ctl,
virInterfaceFree(ifaces[i]);
VIR_FREE(ifaces);
return ret;
-
- error:
- goto cleanup;
}
@@ -377,13 +363,13 @@ virshNetworkNameCompleter(vshControl *ctl,
return NULL;
if (VIR_ALLOC_N(tmp, nnets + 1) < 0)
- goto error;
+ goto cleanup;
for (i = 0; i < nnets; i++) {
const char *name = virNetworkGetName(nets[i]);
if (VIR_STRDUP(tmp[i], name) < 0)
- goto error;
+ goto cleanup;
}
VIR_STEAL_PTR(ret, tmp);
@@ -393,9 +379,6 @@ virshNetworkNameCompleter(vshControl *ctl,
virNetworkFree(nets[i]);
VIR_FREE(nets);
return ret;
-
- error:
- goto cleanup;
}
@@ -411,20 +394,17 @@ virshNetworkEventNameCompleter(vshControl *ctl ATTRIBUTE_UNUSED,
virCheckFlags(0, NULL);
if (VIR_ALLOC_N(tmp, VIR_NETWORK_EVENT_ID_LAST + 1) < 0)
- goto error;
+ goto cleanup;
for (i = 0; i < VIR_NETWORK_EVENT_ID_LAST; i++) {
if (VIR_STRDUP(tmp[i], virshNetworkEventCallbacks[i].name) < 0)
- goto error;
+ goto cleanup;
}
VIR_STEAL_PTR(ret, tmp);
cleanup:
return ret;
-
- error:
- goto cleanup;
}
@@ -449,13 +429,13 @@ virshNodeDeviceNameCompleter(vshControl *ctl,
return NULL;
if (VIR_ALLOC_N(tmp, ndevs + 1) < 0)
- goto error;
+ goto cleanup;
for (i = 0; i < ndevs; i++) {
const char *name = virNodeDeviceGetName(devs[i]);
if (VIR_STRDUP(tmp[i], name) < 0)
- goto error;
+ goto cleanup;
}
VIR_STEAL_PTR(ret, tmp);
@@ -465,9 +445,6 @@ virshNodeDeviceNameCompleter(vshControl *ctl,
virNodeDeviceFree(devs[i]);
VIR_FREE(devs);
return ret;
-
- error:
- goto cleanup;
}
@@ -492,13 +469,13 @@ virshNWFilterNameCompleter(vshControl *ctl,
return NULL;
if (VIR_ALLOC_N(tmp, nnwfilters + 1) < 0)
- goto error;
+ goto cleanup;
for (i = 0; i < nnwfilters; i++) {
const char *name = virNWFilterGetName(nwfilters[i]);
if (VIR_STRDUP(tmp[i], name) < 0)
- goto error;
+ goto cleanup;
}
VIR_STEAL_PTR(ret, tmp);
@@ -508,9 +485,6 @@ virshNWFilterNameCompleter(vshControl *ctl,
virNWFilterFree(nwfilters[i]);
VIR_FREE(nwfilters);
return ret;
-
- error:
- goto cleanup;
}
@@ -535,13 +509,13 @@ virshNWFilterBindingNameCompleter(vshControl *ctl,
return NULL;
if (VIR_ALLOC_N(tmp, nbindings + 1) < 0)
- goto error;
+ goto cleanup;
for (i = 0; i < nbindings; i++) {
const char *name = virNWFilterBindingGetPortDev(bindings[i]);
if (VIR_STRDUP(tmp[i], name) < 0)
- goto error;
+ goto cleanup;
}
VIR_STEAL_PTR(ret, tmp);
@@ -551,9 +525,6 @@ virshNWFilterBindingNameCompleter(vshControl *ctl,
virNWFilterBindingFree(bindings[i]);
VIR_FREE(bindings);
return ret;
-
- error:
- goto cleanup;
}
@@ -578,14 +549,14 @@ virshSecretUUIDCompleter(vshControl *ctl,
return NULL;
if (VIR_ALLOC_N(tmp, nsecrets + 1) < 0)
- goto error;
+ goto cleanup;
for (i = 0; i < nsecrets; i++) {
char uuid[VIR_UUID_STRING_BUFLEN];
if (virSecretGetUUIDString(secrets[i], uuid) < 0 ||
VIR_STRDUP(tmp[i], uuid) < 0)
- goto error;
+ goto cleanup;
}
VIR_STEAL_PTR(ret, tmp);
@@ -595,9 +566,6 @@ virshSecretUUIDCompleter(vshControl *ctl,
virSecretFree(secrets[i]);
VIR_FREE(secrets);
return ret;
-
- error:
- goto cleanup;
}
@@ -624,17 +592,17 @@ virshSnapshotNameCompleter(vshControl *ctl,
return NULL;
if ((rc = virDomainListAllSnapshots(dom, &snapshots, flags)) < 0)
- goto error;
+ goto cleanup;
nsnapshots = rc;
if (VIR_ALLOC_N(tmp, nsnapshots + 1) < 0)
- goto error;
+ goto cleanup;
for (i = 0; i < nsnapshots; i++) {
const char *name = virDomainSnapshotGetName(snapshots[i]);
if (VIR_STRDUP(tmp[i], name) < 0)
- goto error;
+ goto cleanup;
}
VIR_STEAL_PTR(ret, tmp);
@@ -645,9 +613,6 @@ virshSnapshotNameCompleter(vshControl *ctl,
virshDomainSnapshotFree(snapshots[i]);
VIR_FREE(snapshots);
return ret;
-
- error:
- goto cleanup;
}
char **
@@ -676,30 +641,30 @@ virshAllocpagesPagesizeCompleter(vshControl *ctl,
virCheckFlags(0, NULL);
if (!priv->conn || virConnectIsAlive(priv->conn) <= 0)
- goto error;
+ return NULL;
if (!(cap_xml = virConnectGetCapabilities(priv->conn)))
- goto error;
+ return NULL;
if (!(doc = virXMLParseStringCtxt(cap_xml, _("capabilities"), &ctxt)))
- goto error;
+ return NULL;
if (cellno && vshCommandOptStringQuiet(ctl, cmd, "cellno",
&cellnum) > 0) {
if (virAsprintf(&path,
"/capabilities/host/topology/cells/cell[@id=\"%s\"]/pages",
cellnum) < 0)
- goto error;
+ return NULL;
} else {
if (virAsprintf(&path, "/capabilities/host/cpu/pages") < 0)
- goto error;
+ return NULL;
}
npages = virXPathNodeSet(path, ctxt, &pages);
if (npages <= 0)
- goto error;
+ return NULL;
if (VIR_ALLOC_N(tmp, npages + 1) < 0)
- goto error;
+ return NULL;
for (i = 0; i < npages; i++) {
VIR_FREE(pagesize);
@@ -707,21 +672,16 @@ virshAllocpagesPagesizeCompleter(vshControl *ctl,
pagesize = virXMLPropString(pages[i], "size");
unit = virXMLPropString(pages[i], "unit");
if (virStrToLong_ull(pagesize, NULL, 10, &byteval) < 0)
- goto error;
+ return NULL;
if (virScaleInteger(&byteval, unit, 1024, UINT_MAX) < 0)
- goto error;
+ return NULL;
size = vshPrettyCapacity(byteval, &suffix);
if (virAsprintf(&tmp[i], "%.0f%s", size, suffix) < 0)
- goto error;
+ return NULL;
}
VIR_STEAL_PTR(ret, tmp);
-
- cleanup:
return ret;
-
- error:
- goto cleanup;
}
@@ -737,20 +697,15 @@ virshSecretEventNameCompleter(vshControl *ctl ATTRIBUTE_UNUSED,
virCheckFlags(0, NULL);
if (VIR_ALLOC_N(tmp, VIR_SECRET_EVENT_ID_LAST + 1) < 0)
- goto error;
+ return NULL;
for (i = 0; i < VIR_SECRET_EVENT_ID_LAST; i++) {
if (VIR_STRDUP(tmp[i], virshSecretEventCallbacks[i].name) < 0)
- goto error;
+ return NULL;
}
VIR_STEAL_PTR(ret, tmp);
-
- cleanup:
return ret;
-
- error:
- goto cleanup;
}
@@ -766,20 +721,15 @@ virshDomainEventNameCompleter(vshControl *ctl ATTRIBUTE_UNUSED,
virCheckFlags(0, NULL);
if (VIR_ALLOC_N(tmp, VIR_DOMAIN_EVENT_ID_LAST + 1) < 0)
- goto error;
+ return NULL;
for (i = 0; i < VIR_DOMAIN_EVENT_ID_LAST; i++) {
if (VIR_STRDUP(tmp[i], virshDomainEventCallbacks[i].name) < 0)
- goto error;
+ return NULL;
}
VIR_STEAL_PTR(ret, tmp);
-
- cleanup:
return ret;
-
- error:
- goto cleanup;
}
@@ -795,20 +745,15 @@ virshPoolEventNameCompleter(vshControl *ctl ATTRIBUTE_UNUSED,
virCheckFlags(0, NULL);
if (VIR_ALLOC_N(tmp, VIR_STORAGE_POOL_EVENT_ID_LAST + 1) < 0)
- goto error;
+ return NULL;
for (i = 0; i < VIR_STORAGE_POOL_EVENT_ID_LAST; i++) {
if (VIR_STRDUP(tmp[i], virshPoolEventCallbacks[i].name) < 0)
- goto error;
+ return NULL;
}
VIR_STEAL_PTR(ret, tmp);
-
- cleanup:
return ret;
-
- error:
- goto cleanup;
}
@@ -836,10 +781,10 @@ virshDomainInterfaceStateCompleter(vshControl *ctl,
return NULL;
if (virshDomainGetXML(ctl, cmd, flags, &xml, &ctxt) < 0)
- goto cleanup;
+ return NULL;
if (vshCommandOptStringReq(ctl, cmd, "interface", &iface) < 0)
- goto cleanup;
+ return NULL;
/* normalize the mac addr */
if (virMacAddrParse(iface, &macaddr) == 0)
@@ -848,35 +793,30 @@ virshDomainInterfaceStateCompleter(vshControl *ctl,
if (virAsprintf(&xpath, "/domain/devices/interface[(mac/@address =
'%s') or "
" (target/@dev =
'%s')]",
macstr, iface) < 0)
- goto cleanup;
+ return NULL;
if ((ninterfaces = virXPathNodeSet(xpath, ctxt, &interfaces)) < 0)
- goto cleanup;
+ return NULL;
if (ninterfaces != 1)
- goto cleanup;
+ return NULL;
ctxt->node = interfaces[0];
if (VIR_ALLOC_N(tmp, 2) < 0)
- goto error;
+ return NULL;
if ((state = virXPathString("string(./link/@state)", ctxt)) &&
STREQ(state, "down")) {
if (VIR_STRDUP(tmp[0], "up") < 0)
- goto error;
+ return NULL;
} else {
if (VIR_STRDUP(tmp[0], "down") < 0)
- goto error;
+ return NULL;
}
VIR_STEAL_PTR(ret, tmp);
-
- cleanup:
return ret;
-
- error:
- goto cleanup;
}
@@ -892,20 +832,15 @@ virshNodedevEventNameCompleter(vshControl *ctl ATTRIBUTE_UNUSED,
virCheckFlags(0, NULL);
if (VIR_ALLOC_N(tmp, VIR_NODE_DEVICE_EVENT_ID_LAST + 1) < 0)
- goto error;
+ return NULL;
for (i = 0; i < VIR_NODE_DEVICE_EVENT_ID_LAST; i++) {
if (VIR_STRDUP(tmp[i], virshNodedevEventCallbacks[i].name) < 0)
- goto error;
+ return NULL;
}
VIR_STEAL_PTR(ret, tmp);
-
- cleanup:
return ret;
-
- error:
- goto cleanup;
}
@@ -927,33 +862,28 @@ virshCellnoCompleter(vshControl *ctl,
virCheckFlags(0, NULL);
if (!priv->conn || virConnectIsAlive(priv->conn) <= 0)
- goto error;
+ return NULL;
if (!(cap_xml = virConnectGetCapabilities(priv->conn)))
- goto error;
+ return NULL;
if (!(doc = virXMLParseStringCtxt(cap_xml, _("capabilities"), &ctxt)))
- goto error;
+ return NULL;
ncells = virXPathNodeSet("/capabilities/host/topology/cells/cell", ctxt,
&cells);
if (ncells <= 0)
- goto error;
+ return NULL;
if (VIR_ALLOC_N(tmp, ncells + 1))
- goto error;
+ return NULL;
for (i = 0; i < ncells; i++) {
if (!(tmp[i] = virXMLPropString(cells[i], "id")))
- goto error;
+ return NULL;
}
VIR_STEAL_PTR(ret, tmp);
-
- cleanup:
return ret;
-
- error:
- goto cleanup;
}
@@ -981,21 +911,20 @@ virshDomainDeviceAliasCompleter(vshControl *ctl,
domainXMLFlags = VIR_DOMAIN_XML_INACTIVE;
if (virshDomainGetXML(ctl, cmd, domainXMLFlags, &xmldoc, &ctxt) < 0)
- goto cleanup;
+ return NULL;
naliases = virXPathNodeSet("./devices//alias/@name", ctxt, &aliases);
if (naliases < 0)
- goto cleanup;
+ return NULL;
if (VIR_ALLOC_N(tmp, naliases + 1) < 0)
- goto cleanup;
+ return NULL;
for (i = 0; i < naliases; i++) {
if (!(tmp[i] = virXMLNodeContentString(aliases[i])))
- goto cleanup;
+ return NULL;
}
VIR_STEAL_PTR(ret, tmp);
- cleanup:
return ret;
}
--
2.20.1
4:05 p.m.
New subject: [libvirt] [PATCH 12/14] virsh-completer: remove excessive labels
On Mon, Apr 01, 2019 at 09:33:29 +0200, Ján Tomko wrote:
Now that we have a shared cleanup section everywhere,
delete all the 'error' labels which all contain just 'goto cleanup'
anyway.
Also remove all the 'cleanup' labels that only 'return ret' - we
can simply return NULL instead of jumping to that label.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
ACK
3:33 p.m.
New subject: [libvirt] [PATCH 13/14] virsh-completer: introduce virshPagesizeNodeToString
A helper function that takes a XML node with a "size"
and "unit" attributes and converts it into a human-readable string.
Reduce the size and number of variables in the parent function.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
tools/virsh-completer.c | 38 +++++++++++++++++++++++---------------
1 file changed, 23 insertions(+), 15 deletions(-)
diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c
index 2b9749300a..5985f09272 100644
--- a/tools/virsh-completer.c
+++ b/tools/virsh-completer.c
@@ -615,27 +615,44 @@ virshSnapshotNameCompleter(vshControl *ctl,
return ret;
}
+static char *
+virshPagesizeNodeToString(xmlNodePtr node)
+{
+ VIR_AUTOFREE(char *) pagesize = NULL;
+ VIR_AUTOFREE(char *) unit = NULL;
+ unsigned long long byteval = 0;
+ const char *suffix = NULL;
+ double size = 0;
+ char *ret;
+
+ pagesize = virXMLPropString(node, "size");
+ unit = virXMLPropString(node, "unit");
+ if (virStrToLong_ull(pagesize, NULL, 10, &byteval) < 0)
+ return NULL;
+ if (virScaleInteger(&byteval, unit, 1024, UINT_MAX) < 0)
+ return NULL;
+ size = vshPrettyCapacity(byteval, &suffix);
+ if (virAsprintf(&ret, "%.0f%s", size, suffix) < 0)
+ return NULL;
+ return ret;
+}
+
char **
virshAllocpagesPagesizeCompleter(vshControl *ctl,
const vshCmd *cmd ATTRIBUTE_UNUSED,
unsigned int flags)
{
- unsigned long long byteval = 0;
VIR_AUTOPTR(xmlXPathContext) ctxt = NULL;
virshControlPtr priv = ctl->privData;
unsigned int npages = 0;
VIR_AUTOFREE(xmlNodePtr *) pages = NULL;
VIR_AUTOPTR(xmlDoc) doc = NULL;
- double size = 0;
size_t i = 0;
- const char *suffix = NULL;
const char *cellnum = NULL;
bool cellno = vshCommandOptBool(cmd, "cellno");
VIR_AUTOFREE(char *) path = NULL;
- VIR_AUTOFREE(char *) pagesize = NULL;
VIR_AUTOFREE(char *) cap_xml = NULL;
char **ret = NULL;
- VIR_AUTOFREE(char *) unit = NULL;
VIR_AUTOSTRINGLIST tmp = NULL;
virCheckFlags(0, NULL);
@@ -667,16 +684,7 @@ virshAllocpagesPagesizeCompleter(vshControl *ctl,
return NULL;
for (i = 0; i < npages; i++) {
- VIR_FREE(pagesize);
- VIR_FREE(unit);
- pagesize = virXMLPropString(pages[i], "size");
- unit = virXMLPropString(pages[i], "unit");
- if (virStrToLong_ull(pagesize, NULL, 10, &byteval) < 0)
- return NULL;
- if (virScaleInteger(&byteval, unit, 1024, UINT_MAX) < 0)
- return NULL;
- size = vshPrettyCapacity(byteval, &suffix);
- if (virAsprintf(&tmp[i], "%.0f%s", size, suffix) < 0)
+ if (!(tmp[i] = virshPagesizeNodeToString(pages[i])))
return NULL;
}
--
2.20.1
4:07 p.m.
New subject: [libvirt] [PATCH 13/14] virsh-completer: introduce virshPagesizeNodeToString
On Mon, Apr 01, 2019 at 09:33:30 +0200, Ján Tomko wrote:
A helper function that takes a XML node with a "size"
and "unit" attributes and converts it into a human-readable string.
Reduce the size and number of variables in the parent function.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
tools/virsh-completer.c | 38 +++++++++++++++++++++++---------------
1 file changed, 23 insertions(+), 15 deletions(-)
ACK
3:33 p.m.
New subject: [libvirt] [PATCH 14/14] virsh-completer: introduce virshPasswordCompleter
Suggest some passwords to the user.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
tools/virsh-completer.c | 58 +++++++++++++++++++++++++++++++++++++++++
tools/virsh-completer.h | 4 +++
tools/virsh-domain.c | 1 +
3 files changed, 63 insertions(+)
diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c
index 5985f09272..0687670d37 100644
--- a/tools/virsh-completer.c
+++ b/tools/virsh-completer.c
@@ -32,6 +32,7 @@
#include "virutil.h"
#include "viralloc.h"
#include "virmacaddr.h"
+#include "virrandom.h"
#include "virstring.h"
#include "virxml.h"
@@ -936,3 +937,60 @@ virshDomainDeviceAliasCompleter(vshControl *ctl,
VIR_STEAL_PTR(ret, tmp);
return ret;
}
+
+
+const char *builtin_passwords[] = {
+ "hunter2", /* ******* */
+ "nbusr123", /* Keď nevieš, tak nefušuj */
+ "4ezgi4",
+};
+
+
+char **
+virshPasswordCompleter(vshControl *ctl ATTRIBUTE_UNUSED,
+ const vshCmd *cmd ATTRIBUTE_UNUSED,
+ unsigned int flags)
+{
+ VIR_AUTOFREE(char *) base64 = NULL;
+ VIR_AUTOFREE(unsigned char *) rand = NULL;
+ VIR_AUTOSTRINGLIST tmp = NULL;
+ const size_t optimal_passlen = 8; /* ought to be enough */
+ const char *prefix = NULL;
+ const size_t num = 1;
+ char **ret = NULL;
+ size_t missing;
+ size_t i;
+
+ virCheckFlags(0, NULL);
+
+ if (VIR_ALLOC_N(tmp, num + ARRAY_CARDINALITY(builtin_passwords) + 1) < 0)
+ return NULL;
+
+ ignore_value(vshCommandOptStringQuiet(ctl, cmd, "password", &prefix));
+ if (STREQ_NULLABLE(prefix, " "))
+ prefix = NULL;
+
+ missing = optimal_passlen - MIN(strlen(NULLSTR_EMPTY(prefix)), optimal_passlen);
+
+ if (VIR_ALLOC_N(rand, 7) < 0)
+ return NULL;
+
+ if (virRandomBytes(rand, 6) < 0)
+ return NULL;
+
+ if (!(base64 = virStringEncodeBase64(rand, 6)))
+ return NULL;
+
+ base64[missing] = '\0';
+
+ if (virAsprintf(&tmp[0], "%s%s", NULLSTR_EMPTY(prefix), base64) <
0)
+ return NULL;
+
+ for (i = 0; i < ARRAY_CARDINALITY(builtin_passwords); i++) {
+ if (VIR_STRDUP(tmp[i + 1], builtin_passwords[i]) < 0)
+ return NULL;
+ }
+
+ VIR_STEAL_PTR(ret, tmp);
+ return ret;
+}
diff --git a/tools/virsh-completer.h b/tools/virsh-completer.h
index 2e2e1edafb..d47a5f4da6 100644
--- a/tools/virsh-completer.h
+++ b/tools/virsh-completer.h
@@ -110,4 +110,8 @@ char ** virshDomainDeviceAliasCompleter(vshControl *ctl,
char ** virshCellnoCompleter(vshControl *ctl,
const vshCmd *cmd,
unsigned int flags);
+
+char ** virshPasswordCompleter(vshControl *ctl,
+ const vshCmd *cmd,
+ unsigned int flags);
#endif /* LIBVIRT_VIRSH_COMPLETER_H */
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index e8d5404acf..d8978f5bd1 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -5732,6 +5732,7 @@ static const vshCmdOptDef opts_set_user_password[] = {
{.name = "password",
.type = VSH_OT_DATA,
.flags = VSH_OFLAG_REQ,
+ .completer = virshPasswordCompleter,
.help = N_("the new password")
},
{.name = "encrypted",
--
2.20.1
5:14 p.m.
New subject: [libvirt] [PATCH 14/14] virsh-completer: introduce virshPasswordCompleter
On Mon, Apr 01, 2019 at 09:33:31AM +0200, Ján Tomko wrote:
Suggest some passwords to the user.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
tools/virsh-completer.c | 58 +++++++++++++++++++++++++++++++++++++++++
tools/virsh-completer.h | 4 +++
tools/virsh-domain.c | 1 +
3 files changed, 63 insertions(+)
diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c
index 5985f09272..0687670d37 100644
--- a/tools/virsh-completer.c
+++ b/tools/virsh-completer.c
@@ -32,6 +32,7 @@
#include "virutil.h"
#include "viralloc.h"
#include "virmacaddr.h"
+#include "virrandom.h"
#include "virstring.h"
#include "virxml.h"
@@ -936,3 +937,60 @@ virshDomainDeviceAliasCompleter(vshControl *ctl,
VIR_STEAL_PTR(ret, tmp);
return ret;
}
+
+
+const char *builtin_passwords[] = {
+ "hunter2", /* ******* */
+ "nbusr123", /* Keď nevieš, tak nefušuj */
+ "4ezgi4",
+};
This is quite a limited list of paswords. I think it would be useful to
expand it with the password dump from haveibeenpwned.com The main
problem is that the overhead of a static array with 500,000,000 passwords
might make libvirt packages too large. RPM used to have problems with
packages larger than 2 GB, so not sure how well it will handle 11 GB
RPMs. There could be a negative impact on memory usage when running libvirt,
though virt hosts usually have lots of RAM, so reserving 11 GB for virsh
shouldn't be too big a problem.
+
+
+char **
+virshPasswordCompleter(vshControl *ctl ATTRIBUTE_UNUSED,
+ const vshCmd *cmd ATTRIBUTE_UNUSED,
+ unsigned int flags)
+{
+ VIR_AUTOFREE(char *) base64 = NULL;
+ VIR_AUTOFREE(unsigned char *) rand = NULL;
+ VIR_AUTOSTRINGLIST tmp = NULL;
+ const size_t optimal_passlen = 8; /* ought to be enough */
+ const char *prefix = NULL;
+ const size_t num = 1;
+ char **ret = NULL;
+ size_t missing;
+ size_t i;
+
+ virCheckFlags(0, NULL);
+
+ if (VIR_ALLOC_N(tmp, num + ARRAY_CARDINALITY(builtin_passwords) + 1) < 0)
+ return NULL;
+
+ ignore_value(vshCommandOptStringQuiet(ctl, cmd, "password",
&prefix));
+ if (STREQ_NULLABLE(prefix, " "))
+ prefix = NULL;
+
+ missing = optimal_passlen - MIN(strlen(NULLSTR_EMPTY(prefix)), optimal_passlen);
+
+ if (VIR_ALLOC_N(rand, 7) < 0)
+ return NULL;
+
+ if (virRandomBytes(rand, 6) < 0)
+ return NULL;
+
+ if (!(base64 = virStringEncodeBase64(rand, 6)))
+ return NULL;
+
+ base64[missing] = '\0';
+
+ if (virAsprintf(&tmp[0], "%s%s", NULLSTR_EMPTY(prefix), base64) <
0)
+ return NULL;
+
+ for (i = 0; i < ARRAY_CARDINALITY(builtin_passwords); i++) {
+ if (VIR_STRDUP(tmp[i + 1], builtin_passwords[i]) < 0)
+ return NULL;
Hmm, so an 11 GB static password list will need another 11GB of heap
allocation. This is getting quite inefficient at scale.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
6:31 p.m.
New subject: [libvirt] [PATCH 14/14] virsh-completer: introduce virshPasswordCompleter
+
+
+const char *builtin_passwords[] = {
+ "hunter2", /* ******* */
+ "nbusr123", /* Keď nevieš, tak nefušuj */
^^^^you didn't :D :D :D :D, epic!
For those who are wondering what ^this gem is, Slovakia's National
Security Authority (slovak acronym NBU) used this ultrahard password
on their servers until there was an incident where hackers
used that pw to (with the login combo of "nbusr") break essentially
into every machine on the subnet, downloaded almost 20GB of classified
documents, emails, backups, DB dumps, etc.
Erik
10:09 p.m.
New subject: [libvirt] [PATCH 14/14] virsh-completer: introduce virshPasswordCompleter
On 4/1/19 3:33 AM, Ján Tomko wrote:
Suggest some passwords to the user.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
tools/virsh-completer.c | 58 +++++++++++++++++++++++++++++++++++++++++
tools/virsh-completer.h | 4 +++
tools/virsh-domain.c | 1 +
3 files changed, 63 insertions(+)
diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c
index 5985f09272..0687670d37 100644
--- a/tools/virsh-completer.c
+++ b/tools/virsh-completer.c
@@ -32,6 +32,7 @@
#include "virutil.h"
#include "viralloc.h"
#include "virmacaddr.h"
+#include "virrandom.h"
#include "virstring.h"
#include "virxml.h"
@@ -936,3 +937,60 @@ virshDomainDeviceAliasCompleter(vshControl *ctl,
VIR_STEAL_PTR(ret, tmp);
return ret;
}
+
+
+const char *builtin_passwords[] = {
+ "*******", /* ******* */
+ "********", /* Keď nevieš, tak nefušuj */
+ "******",
+};
I hope that git push doesn't perform the same redaction as git
send-email did (or perhaps the server end of git is equipped to reverse
it?) (This "feature" of git send-email makes it hard for us to judge the
usefulness of the list of recommendations BTW.)
2038
days inactive
2040
days old
30 comments
5 participants
participants (5)
-
Daniel P. Berrangé -
Erik Skultety -
Ján Tomko -
Laine Stump -
Peter Krempa