12:07 a.m.
In this patchset I'm switching from virXXXFree to g_autoptr(). There are
still some left, but very rare occurrence:
libvirt.git $ git grep -o "vir[A-Z].*Free" -- tools/ | \
cut -d':' -f 2 | sort | uniq -c | sort -n
And of course, after these some functions could use subsequent cleanup,
e.g. because cleanup label collapsed to just return statement. But I
leave those for future work.
Michal Prívozník (8):
virsh-util.h: Fix ordering of virshXXXFree functions
virsh: Add wrapper for virInterfaceFree
virsh: Add wrapper for virStoragePoolFree
virsh: Add wrapper for virStorageVolFree
virsh: Add wrapper for virNetworkFree
virsh: Add wrapper for virNodeDeviceFree
virsh: Add wrapper for virNWFilterFree
virsh: Add wrapper for virStreamFree
build-aux/syntax-check.mk | 4 +-
tools/virsh-completer-interface.c | 3 +-
tools/virsh-completer-network.c | 8 +--
tools/virsh-completer-nodedev.c | 3 +-
tools/virsh-completer-nwfilter.c | 3 +-
tools/virsh-completer-pool.c | 3 +-
tools/virsh-completer-volume.c | 8 +--
tools/virsh-console.c | 8 +--
tools/virsh-domain.c | 10 +--
tools/virsh-interface.c | 55 +++++----------
tools/virsh-network.c | 75 ++++++--------------
tools/virsh-nodedev.c | 49 ++++----------
tools/virsh-nwfilter.c | 22 ++----
tools/virsh-pool.c | 67 ++++++------------
tools/virsh-util.c | 78 +++++++++++++++++++++
tools/virsh-util.h | 46 +++++++++++--
tools/virsh-volume.c | 109 ++++++++----------------------
17 files changed, 254 insertions(+), 297 deletions(-)
--
2.32.0
12:07 a.m.
New subject: [PATCH 1/8] virsh-util.h: Fix ordering of virshXXXFree functions
Currently the order of virshXXXFree functions in the header file
does not correspond to the order in the corresponding .c file.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
tools/virsh-util.h | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/tools/virsh-util.h b/tools/virsh-util.h
index 6115b05d4d..f7e8116de3 100644
--- a/tools/virsh-util.h
+++ b/tools/virsh-util.h
@@ -40,16 +40,10 @@ virshCommandOptDomain(vshControl *ctl,
const char **name);
typedef virDomain virshDomain;
-
void
virshDomainFree(virDomainPtr dom);
G_DEFINE_AUTOPTR_CLEANUP_FUNC(virshDomain, virshDomainFree);
-typedef virSecret virshSecret;
-void
-virshSecretFree(virSecretPtr secret);
-G_DEFINE_AUTOPTR_CLEANUP_FUNC(virshSecret, virshSecretFree);
-
typedef virDomainCheckpoint virshDomainCheckpoint;
void
virshDomainCheckpointFree(virDomainCheckpointPtr chk);
@@ -60,6 +54,11 @@ void
virshDomainSnapshotFree(virDomainSnapshotPtr snap);
G_DEFINE_AUTOPTR_CLEANUP_FUNC(virshDomainSnapshot, virshDomainSnapshotFree);
+typedef virSecret virshSecret;
+void
+virshSecretFree(virSecretPtr secret);
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(virshSecret, virshSecretFree);
+
int
virshDomainState(vshControl *ctl,
virDomainPtr dom,
--
2.32.0
12:07 a.m.
New subject: [PATCH 2/8] virsh: Add wrapper for virInterfaceFree
Similarly to virshDomainFree add a wrapper for the snapshot object
freeing function.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
build-aux/syntax-check.mk | 4 +--
tools/virsh-completer-interface.c | 3 +-
tools/virsh-interface.c | 55 +++++++++----------------------
tools/virsh-util.c | 11 +++++++
tools/virsh-util.h | 5 +++
5 files changed, 36 insertions(+), 42 deletions(-)
diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk
index cb54c8ba36..84cb895d86 100644
--- a/build-aux/syntax-check.mk
+++ b/build-aux/syntax-check.mk
@@ -868,10 +868,10 @@ sc_gettext_init:
$(_sc_search_regexp)
sc_prohibit_obj_free_apis_in_virsh:
- @prohibit='\bvir(Domain|DomainSnapshot|Secret)Free\b' \
+ @prohibit='\bvir(Domain|DomainSnapshot|Interface|Secret)Free\b' \
in_vc_files='virsh.*\.[ch]$$' \
exclude='sc_prohibit_obj_free_apis_in_virsh' \
- halt='avoid using virDomain(Snapshot)Free in virsh, use virsh-prefixed wrappers
instead' \
+ halt='avoid using public virXXXFree in virsh, use virsh-prefixed wrappers
instead' \
$(_sc_search_regexp)
https_sites = www.libvirt.org
diff --git a/tools/virsh-completer-interface.c b/tools/virsh-completer-interface.c
index 9a6603b27e..44ba35550a 100644
--- a/tools/virsh-completer-interface.c
+++ b/tools/virsh-completer-interface.c
@@ -21,6 +21,7 @@
#include <config.h>
#include "virsh-completer-interface.h"
+#include "virsh-util.h"
#include "viralloc.h"
#include "virsh.h"
#include "virstring.h"
@@ -59,7 +60,7 @@ virshInterfaceStringHelper(vshControl *ctl,
}
for (i = 0; i < nifaces; i++)
- virInterfaceFree(ifaces[i]);
+ virshInterfaceFree(ifaces[i]);
g_free(ifaces);
return g_steal_pointer(&tmp);
diff --git a/tools/virsh-interface.c b/tools/virsh-interface.c
index f402119b68..0a8539da71 100644
--- a/tools/virsh-interface.c
+++ b/tools/virsh-interface.c
@@ -29,6 +29,7 @@
#include <config.h>
#include "virsh-interface.h"
+#include "virsh-util.h"
#include <libxml/parser.h>
#include <libxml/xpath.h>
@@ -109,8 +110,8 @@ static bool
cmdInterfaceEdit(vshControl *ctl, const vshCmd *cmd)
{
bool ret = false;
- virInterfacePtr iface = NULL;
- virInterfacePtr iface_edited = NULL;
+ g_autoptr(virshInterface) iface = NULL;
+ g_autoptr(virshInterface) iface_edited = NULL;
unsigned int flags = VIR_INTERFACE_XML_INACTIVE;
virshControl *priv = ctl->privData;
@@ -136,11 +137,6 @@ cmdInterfaceEdit(vshControl *ctl, const vshCmd *cmd)
ret = true;
cleanup:
- if (iface)
- virInterfaceFree(iface);
- if (iface_edited)
- virInterfaceFree(iface_edited);
-
return ret;
}
@@ -172,8 +168,7 @@ virshInterfaceListFree(struct virshInterfaceList *list)
if (list && list->ifaces) {
for (i = 0; i < list->nifaces; i++) {
- if (list->ifaces[i])
- virInterfaceFree(list->ifaces[i]);
+ virshInterfaceFree(list->ifaces[i]);
}
g_free(list->ifaces);
}
@@ -411,14 +406,13 @@ static const vshCmdOptDef opts_interface_name[] = {
static bool
cmdInterfaceName(vshControl *ctl, const vshCmd *cmd)
{
- virInterfacePtr iface;
+ g_autoptr(virshInterface) iface = NULL;
if (!(iface = virshCommandOptInterfaceBy(ctl, cmd, NULL, NULL,
VIRSH_BYMAC)))
return false;
vshPrint(ctl, "%s\n", virInterfaceGetName(iface));
- virInterfaceFree(iface);
return true;
}
@@ -448,14 +442,13 @@ static const vshCmdOptDef opts_interface_mac[] = {
static bool
cmdInterfaceMAC(vshControl *ctl, const vshCmd *cmd)
{
- virInterfacePtr iface;
+ g_autoptr(virshInterface) iface = NULL;
if (!(iface = virshCommandOptInterfaceBy(ctl, cmd, NULL, NULL,
VIRSH_BYNAME)))
return false;
vshPrint(ctl, "%s\n", virInterfaceGetMACString(iface));
- virInterfaceFree(iface);
return true;
}
@@ -484,7 +477,7 @@ static const vshCmdOptDef opts_interface_dumpxml[] = {
static bool
cmdInterfaceDumpXML(vshControl *ctl, const vshCmd *cmd)
{
- virInterfacePtr iface;
+ g_autoptr(virshInterface) iface = NULL;
g_autofree char *dump = NULL;
unsigned int flags = 0;
@@ -494,13 +487,10 @@ cmdInterfaceDumpXML(vshControl *ctl, const vshCmd *cmd)
if (!(iface = virshCommandOptInterface(ctl, cmd, NULL)))
return false;
- if (!(dump = virInterfaceGetXMLDesc(iface, flags))) {
- virInterfaceFree(iface);
+ if (!(dump = virInterfaceGetXMLDesc(iface, flags)))
return false;
- }
vshPrint(ctl, "%s", dump);
- virInterfaceFree(iface);
return true;
}
@@ -530,7 +520,7 @@ static const vshCmdOptDef opts_interface_define[] = {
static bool
cmdInterfaceDefine(vshControl *ctl, const vshCmd *cmd)
{
- virInterfacePtr iface;
+ g_autoptr(virshInterface) iface = NULL;
const char *from = NULL;
g_autofree char *buffer = NULL;
unsigned int flags = 0;
@@ -552,7 +542,6 @@ cmdInterfaceDefine(vshControl *ctl, const vshCmd *cmd)
vshPrintExtra(ctl, _("Interface %s defined from %s\n"),
virInterfaceGetName(iface), from);
- virInterfaceFree(iface);
return true;
}
@@ -577,7 +566,7 @@ static const vshCmdOptDef opts_interface_undefine[] = {
static bool
cmdInterfaceUndefine(vshControl *ctl, const vshCmd *cmd)
{
- virInterfacePtr iface;
+ g_autoptr(virshInterface) iface = NULL;
const char *name;
if (!(iface = virshCommandOptInterface(ctl, cmd, &name)))
@@ -585,12 +574,10 @@ cmdInterfaceUndefine(vshControl *ctl, const vshCmd *cmd)
if (virInterfaceUndefine(iface) < 0) {
vshError(ctl, _("Failed to undefine interface %s"), name);
- virInterfaceFree(iface);
return false;
}
vshPrintExtra(ctl, _("Interface %s undefined\n"), name);
- virInterfaceFree(iface);
return true;
}
@@ -615,7 +602,7 @@ static const vshCmdOptDef opts_interface_start[] = {
static bool
cmdInterfaceStart(vshControl *ctl, const vshCmd *cmd)
{
- virInterfacePtr iface;
+ g_autoptr(virshInterface) iface = NULL;
const char *name;
if (!(iface = virshCommandOptInterface(ctl, cmd, &name)))
@@ -623,12 +610,10 @@ cmdInterfaceStart(vshControl *ctl, const vshCmd *cmd)
if (virInterfaceCreate(iface, 0) < 0) {
vshError(ctl, _("Failed to start interface %s"), name);
- virInterfaceFree(iface);
return false;
}
vshPrintExtra(ctl, _("Interface %s started\n"), name);
- virInterfaceFree(iface);
return true;
}
@@ -653,7 +638,7 @@ static const vshCmdOptDef opts_interface_destroy[] = {
static bool
cmdInterfaceDestroy(vshControl *ctl, const vshCmd *cmd)
{
- virInterfacePtr iface;
+ g_autoptr(virshInterface) iface = NULL;
const char *name;
if (!(iface = virshCommandOptInterface(ctl, cmd, &name)))
@@ -661,12 +646,10 @@ cmdInterfaceDestroy(vshControl *ctl, const vshCmd *cmd)
if (virInterfaceDestroy(iface, 0) < 0) {
vshError(ctl, _("Failed to destroy interface %s"), name);
- virInterfaceFree(iface);
return false;
}
vshPrintExtra(ctl, _("Interface %s destroyed\n"), name);
- virInterfaceFree(iface);
return false;
}
@@ -809,7 +792,8 @@ static bool
cmdInterfaceBridge(vshControl *ctl, const vshCmd *cmd)
{
bool ret = false;
- virInterfacePtr if_handle = NULL, br_handle = NULL;
+ g_autoptr(virshInterface) if_handle = NULL;
+ g_autoptr(virshInterface) br_handle = NULL;
const char *if_name, *br_name;
char *if_type = NULL, *if2_name = NULL, *delay_str = NULL;
bool stp = false, nostart = false;
@@ -988,10 +972,6 @@ cmdInterfaceBridge(vshControl *ctl, const vshCmd *cmd)
ret = true;
cleanup:
- if (if_handle)
- virInterfaceFree(if_handle);
- if (br_handle)
- virInterfaceFree(br_handle);
VIR_FREE(if_xml);
VIR_FREE(br_xml);
VIR_FREE(if_type);
@@ -1030,7 +1010,8 @@ static bool
cmdInterfaceUnbridge(vshControl *ctl, const vshCmd *cmd)
{
bool ret = false;
- virInterfacePtr if_handle = NULL, br_handle = NULL;
+ g_autoptr(virshInterface) if_handle = NULL;
+ g_autoptr(virshInterface) br_handle = NULL;
const char *br_name;
char *if_type = NULL, *if_name = NULL;
bool nostart = false;
@@ -1187,10 +1168,6 @@ cmdInterfaceUnbridge(vshControl *ctl, const vshCmd *cmd)
ret = true;
cleanup:
- if (if_handle)
- virInterfaceFree(if_handle);
- if (br_handle)
- virInterfaceFree(br_handle);
VIR_FREE(if_xml);
VIR_FREE(br_xml);
VIR_FREE(if_type);
diff --git a/tools/virsh-util.c b/tools/virsh-util.c
index fb74514b3c..82523f2575 100644
--- a/tools/virsh-util.c
+++ b/tools/virsh-util.c
@@ -285,6 +285,17 @@ virshDomainSnapshotFree(virDomainSnapshotPtr snap)
}
+void
+virshInterfaceFree(virInterfacePtr iface)
+{
+ if (!iface)
+ return;
+
+ vshSaveLibvirtHelperError();
+ virInterfaceFree(iface); /* sc_prohibit_obj_free_apis_in_virsh */
+}
+
+
void
virshSecretFree(virSecretPtr secret)
{
diff --git a/tools/virsh-util.h b/tools/virsh-util.h
index f7e8116de3..7165755550 100644
--- a/tools/virsh-util.h
+++ b/tools/virsh-util.h
@@ -54,6 +54,11 @@ void
virshDomainSnapshotFree(virDomainSnapshotPtr snap);
G_DEFINE_AUTOPTR_CLEANUP_FUNC(virshDomainSnapshot, virshDomainSnapshotFree);
+typedef virInterface virshInterface;
+void
+virshInterfaceFree(virInterfacePtr iface);
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(virshInterface, virshInterfaceFree);
+
typedef virSecret virshSecret;
void
virshSecretFree(virSecretPtr secret);
--
2.32.0
12:07 a.m.
New subject: [PATCH 3/8] virsh: Add wrapper for virStoragePoolFree
Similarly to virshDomainFree add a wrapper for the snapshot object
freeing function.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
build-aux/syntax-check.mk | 2 +-
tools/virsh-completer-pool.c | 3 +-
tools/virsh-completer-volume.c | 4 +-
tools/virsh-domain.c | 3 +-
tools/virsh-pool.c | 67 +++++++++++-----------------------
tools/virsh-util.c | 11 ++++++
tools/virsh-util.h | 5 +++
tools/virsh-volume.c | 29 ++++-----------
8 files changed, 51 insertions(+), 73 deletions(-)
diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk
index 84cb895d86..111d2109e8 100644
--- a/build-aux/syntax-check.mk
+++ b/build-aux/syntax-check.mk
@@ -868,7 +868,7 @@ sc_gettext_init:
$(_sc_search_regexp)
sc_prohibit_obj_free_apis_in_virsh:
- @prohibit='\bvir(Domain|DomainSnapshot|Interface|Secret)Free\b' \
+ @prohibit='\bvir(Domain|DomainSnapshot|Interface|Secret|StoragePool)Free\b' \
in_vc_files='virsh.*\.[ch]$$' \
exclude='sc_prohibit_obj_free_apis_in_virsh' \
halt='avoid using public virXXXFree in virsh, use virsh-prefixed wrappers
instead' \
diff --git a/tools/virsh-completer-pool.c b/tools/virsh-completer-pool.c
index 9350eff2d3..84e9d6cc5a 100644
--- a/tools/virsh-completer-pool.c
+++ b/tools/virsh-completer-pool.c
@@ -21,6 +21,7 @@
#include <config.h>
#include "virsh-completer-pool.h"
+#include "virsh-util.h"
#include "conf/storage_conf.h"
#include "viralloc.h"
#include "virsh-pool.h"
@@ -61,7 +62,7 @@ virshStoragePoolNameCompleter(vshControl *ctl,
ret = g_steal_pointer(&tmp);
for (i = 0; i < npools; i++)
- virStoragePoolFree(pools[i]);
+ virshStoragePoolFree(pools[i]);
g_free(pools);
return ret;
}
diff --git a/tools/virsh-completer-volume.c b/tools/virsh-completer-volume.c
index fcbc28b13b..1d83643c69 100644
--- a/tools/virsh-completer-volume.c
+++ b/tools/virsh-completer-volume.c
@@ -21,6 +21,7 @@
#include <config.h>
#include "virsh-completer-volume.h"
+#include "virsh-util.h"
#include "viralloc.h"
#include "virsh-pool.h"
#include "virsh.h"
@@ -32,7 +33,7 @@ virshStorageVolNameCompleter(vshControl *ctl,
unsigned int flags)
{
virshControl *priv = ctl->privData;
- virStoragePoolPtr pool = NULL;
+ g_autoptr(virshStoragePool) pool = NULL;
virStorageVolPtr *vols = NULL;
int rc;
int nvols = 0;
@@ -63,7 +64,6 @@ virshStorageVolNameCompleter(vshControl *ctl,
ret = g_steal_pointer(&tmp);
cleanup:
- virStoragePoolFree(pool);
for (i = 0; i < nvols; i++)
virStorageVolFree(vols[i]);
g_free(vols);
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 0b78fbf728..461a5e19f6 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -3801,7 +3801,7 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd)
}
if (pool) {
- virStoragePoolPtr storagepool = NULL;
+ g_autoptr(virshStoragePool) storagepool = NULL;
if (!source) {
vshError(ctl,
@@ -3820,7 +3820,6 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd)
}
vol.vol = virStorageVolLookupByName(storagepool, source);
- virStoragePoolFree(storagepool);
} else {
vol.vol = virStorageVolLookupByPath(priv->conn, source);
diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c
index fd9d5ead63..d391257f6e 100644
--- a/tools/virsh-pool.c
+++ b/tools/virsh-pool.c
@@ -20,6 +20,7 @@
#include <config.h>
#include "virsh-pool.h"
+#include "virsh-util.h"
#include "internal.h"
#include "virbuffer.h"
@@ -219,7 +220,7 @@ static const vshCmdOptDef opts_pool_autostart[] = {
static bool
cmdPoolAutostart(vshControl *ctl, const vshCmd *cmd)
{
- virStoragePoolPtr pool;
+ g_autoptr(virshStoragePool) pool = NULL;
const char *name;
int autostart;
@@ -233,7 +234,6 @@ cmdPoolAutostart(vshControl *ctl, const vshCmd *cmd)
vshError(ctl, _("failed to mark pool %s as autostarted"), name);
else
vshError(ctl, _("failed to unmark pool %s as autostarted"), name);
- virStoragePoolFree(pool);
return false;
}
@@ -242,7 +242,6 @@ cmdPoolAutostart(vshControl *ctl, const vshCmd *cmd)
else
vshPrintExtra(ctl, _("Pool %s unmarked as autostarted\n"), name);
- virStoragePoolFree(pool);
return true;
}
@@ -271,7 +270,7 @@ static const vshCmdOptDef opts_pool_create[] = {
static bool
cmdPoolCreate(vshControl *ctl, const vshCmd *cmd)
{
- virStoragePoolPtr pool;
+ g_autoptr(virshStoragePool) pool = NULL;
const char *from = NULL;
g_autofree char *buffer = NULL;
bool build;
@@ -307,7 +306,6 @@ cmdPoolCreate(vshControl *ctl, const vshCmd *cmd)
vshPrintExtra(ctl, _("Pool %s created from %s\n"),
virStoragePoolGetName(pool), from);
- virStoragePoolFree(pool);
return true;
}
@@ -462,7 +460,7 @@ static const vshCmdOptDef opts_pool_create_as[] = {
static bool
cmdPoolCreateAs(vshControl *ctl, const vshCmd *cmd)
{
- virStoragePoolPtr pool;
+ g_autoptr(virshStoragePool) pool = NULL;
const char *name;
g_autofree char *xml = NULL;
bool printXML = vshCommandOptBool(cmd, "print-xml");
@@ -500,7 +498,6 @@ cmdPoolCreateAs(vshControl *ctl, const vshCmd *cmd)
}
vshPrintExtra(ctl, _("Pool %s created\n"), name);
- virStoragePoolFree(pool);
return true;
}
@@ -530,7 +527,7 @@ static const vshCmdOptDef opts_pool_define[] = {
static bool
cmdPoolDefine(vshControl *ctl, const vshCmd *cmd)
{
- virStoragePoolPtr pool;
+ g_autoptr(virshStoragePool) pool = NULL;
const char *from = NULL;
g_autofree char *buffer = NULL;
unsigned int flags = 0;
@@ -552,7 +549,6 @@ cmdPoolDefine(vshControl *ctl, const vshCmd *cmd)
vshPrintExtra(ctl, _("Pool %s defined from %s\n"),
virStoragePoolGetName(pool), from);
- virStoragePoolFree(pool);
return true;
}
@@ -572,7 +568,7 @@ static const vshCmdInfo info_pool_define_as[] = {
static bool
cmdPoolDefineAs(vshControl *ctl, const vshCmd *cmd)
{
- virStoragePoolPtr pool;
+ g_autoptr(virshStoragePool) pool = NULL;
const char *name;
g_autofree char *xml = NULL;
bool printXML = vshCommandOptBool(cmd, "print-xml");
@@ -592,7 +588,6 @@ cmdPoolDefineAs(vshControl *ctl, const vshCmd *cmd)
}
vshPrintExtra(ctl, _("Pool %s defined\n"), name);
- virStoragePoolFree(pool);
return true;
}
@@ -620,7 +615,7 @@ static const vshCmdOptDef opts_pool_build[] = {
static bool
cmdPoolBuild(vshControl *ctl, const vshCmd *cmd)
{
- virStoragePoolPtr pool;
+ g_autoptr(virshStoragePool) pool = NULL;
bool ret = true;
const char *name;
unsigned int flags = 0;
@@ -641,8 +636,6 @@ cmdPoolBuild(vshControl *ctl, const vshCmd *cmd)
ret = false;
}
- virStoragePoolFree(pool);
-
return ret;
}
@@ -668,7 +661,7 @@ static const vshCmdOptDef opts_pool_destroy[] = {
static bool
cmdPoolDestroy(vshControl *ctl, const vshCmd *cmd)
{
- virStoragePoolPtr pool;
+ g_autoptr(virshStoragePool) pool = NULL;
bool ret = true;
const char *name;
@@ -682,7 +675,6 @@ cmdPoolDestroy(vshControl *ctl, const vshCmd *cmd)
ret = false;
}
- virStoragePoolFree(pool);
return ret;
}
@@ -708,7 +700,7 @@ static const vshCmdOptDef opts_pool_delete[] = {
static bool
cmdPoolDelete(vshControl *ctl, const vshCmd *cmd)
{
- virStoragePoolPtr pool;
+ g_autoptr(virshStoragePool) pool = NULL;
bool ret = true;
const char *name;
@@ -722,7 +714,6 @@ cmdPoolDelete(vshControl *ctl, const vshCmd *cmd)
ret = false;
}
- virStoragePoolFree(pool);
return ret;
}
@@ -748,7 +739,7 @@ static const vshCmdOptDef opts_pool_refresh[] = {
static bool
cmdPoolRefresh(vshControl *ctl, const vshCmd *cmd)
{
- virStoragePoolPtr pool;
+ g_autoptr(virshStoragePool) pool = NULL;
bool ret = true;
const char *name;
@@ -761,7 +752,6 @@ cmdPoolRefresh(vshControl *ctl, const vshCmd *cmd)
vshError(ctl, _("Failed to refresh pool %s"), name);
ret = false;
}
- virStoragePoolFree(pool);
return ret;
}
@@ -792,7 +782,7 @@ static const vshCmdOptDef opts_pool_dumpxml[] = {
static bool
cmdPoolDumpXML(vshControl *ctl, const vshCmd *cmd)
{
- virStoragePoolPtr pool;
+ g_autoptr(virshStoragePool) pool = NULL;
bool ret = true;
bool inactive = vshCommandOptBool(cmd, "inactive");
unsigned int flags = 0;
@@ -811,7 +801,6 @@ cmdPoolDumpXML(vshControl *ctl, const vshCmd *cmd)
ret = false;
}
- virStoragePoolFree(pool);
return ret;
}
@@ -837,8 +826,7 @@ void virshStoragePoolListFree(struct virshStoragePoolList *list)
if (list && list->pools) {
for (i = 0; i < list->npools; i++) {
- if (list->pools[i])
- virStoragePoolFree(list->pools[i]);
+ virshStoragePoolFree(list->pools[i]);
}
g_free(list->pools);
}
@@ -1003,8 +991,7 @@ virshStoragePoolListCollect(vshControl *ctl,
remove_entry:
/* the pool has to be removed as it failed one of the filters */
- virStoragePoolFree(list->pools[i]);
- list->pools[i] = NULL;
+ g_clear_pointer(&list->pools[i], virshStoragePoolFree);
deleted++;
}
@@ -1570,7 +1557,7 @@ static bool
cmdPoolInfo(vshControl *ctl, const vshCmd *cmd)
{
virStoragePoolInfo info;
- virStoragePoolPtr pool;
+ g_autoptr(virshStoragePool) pool = NULL;
int autostart = 0;
bool ret = true;
bool bytes = false;
@@ -1630,7 +1617,6 @@ cmdPoolInfo(vshControl *ctl, const vshCmd *cmd)
ret = false;
}
- virStoragePoolFree(pool);
return ret;
}
@@ -1656,13 +1642,12 @@ static const vshCmdOptDef opts_pool_name[] = {
static bool
cmdPoolName(vshControl *ctl, const vshCmd *cmd)
{
- virStoragePoolPtr pool;
+ g_autoptr(virshStoragePool) pool = NULL;
if (!(pool = virshCommandOptPoolBy(ctl, cmd, "pool", NULL, VIRSH_BYUUID)))
return false;
vshPrint(ctl, "%s\n", virStoragePoolGetName(pool));
- virStoragePoolFree(pool);
return true;
}
@@ -1691,7 +1676,7 @@ static const vshCmdOptDef opts_pool_start[] = {
static bool
cmdPoolStart(vshControl *ctl, const vshCmd *cmd)
{
- virStoragePoolPtr pool;
+ g_autoptr(virshStoragePool) pool = NULL;
bool ret = true;
const char *name = NULL;
bool build;
@@ -1723,7 +1708,6 @@ cmdPoolStart(vshControl *ctl, const vshCmd *cmd)
ret = false;
}
- virStoragePoolFree(pool);
return ret;
}
@@ -1749,7 +1733,7 @@ static const vshCmdOptDef opts_pool_undefine[] = {
static bool
cmdPoolUndefine(vshControl *ctl, const vshCmd *cmd)
{
- virStoragePoolPtr pool;
+ g_autoptr(virshStoragePool) pool = NULL;
bool ret = true;
const char *name;
@@ -1763,7 +1747,6 @@ cmdPoolUndefine(vshControl *ctl, const vshCmd *cmd)
ret = false;
}
- virStoragePoolFree(pool);
return ret;
}
@@ -1789,7 +1772,7 @@ static const vshCmdOptDef opts_pool_uuid[] = {
static bool
cmdPoolUuid(vshControl *ctl, const vshCmd *cmd)
{
- virStoragePoolPtr pool;
+ g_autoptr(virshStoragePool) pool = NULL;
char uuid[VIR_UUID_STRING_BUFLEN];
if (!(pool = virshCommandOptPoolBy(ctl, cmd, "pool", NULL, VIRSH_BYNAME)))
@@ -1800,7 +1783,6 @@ cmdPoolUuid(vshControl *ctl, const vshCmd *cmd)
else
vshError(ctl, "%s", _("failed to get pool UUID"));
- virStoragePoolFree(pool);
return true;
}
@@ -1827,8 +1809,8 @@ static bool
cmdPoolEdit(vshControl *ctl, const vshCmd *cmd)
{
bool ret = false;
- virStoragePoolPtr pool = NULL;
- virStoragePoolPtr pool_edited = NULL;
+ g_autoptr(virshStoragePool) pool = NULL;
+ g_autoptr(virshStoragePool) pool_edited = NULL;
unsigned int flags = VIR_STORAGE_XML_INACTIVE;
g_autofree char *tmp_desc = NULL;
virshControl *priv = ctl->privData;
@@ -1865,11 +1847,6 @@ cmdPoolEdit(vshControl *ctl, const vshCmd *cmd)
ret = true;
cleanup:
- if (pool)
- virStoragePoolFree(pool);
- if (pool_edited)
- virStoragePoolFree(pool_edited);
-
return ret;
}
@@ -2018,7 +1995,7 @@ static const vshCmdOptDef opts_pool_event[] = {
static bool
cmdPoolEvent(vshControl *ctl, const vshCmd *cmd)
{
- virStoragePoolPtr pool = NULL;
+ g_autoptr(virshStoragePool) pool = NULL;
bool ret = false;
int eventId = -1;
int timeout = 0;
@@ -2088,8 +2065,6 @@ cmdPoolEvent(vshControl *ctl, const vshCmd *cmd)
if (eventId >= 0 &&
virConnectStoragePoolEventDeregisterAny(priv->conn, eventId) < 0)
ret = false;
- if (pool)
- virStoragePoolFree(pool);
return ret;
}
diff --git a/tools/virsh-util.c b/tools/virsh-util.c
index 82523f2575..d537501387 100644
--- a/tools/virsh-util.c
+++ b/tools/virsh-util.c
@@ -307,6 +307,17 @@ virshSecretFree(virSecretPtr secret)
}
+void
+virshStoragePoolFree(virStoragePoolPtr pool)
+{
+ if (!pool)
+ return;
+
+ vshSaveLibvirtHelperError();
+ virStoragePoolFree(pool); /* sc_prohibit_obj_free_apis_in_virsh */
+}
+
+
int
virshDomainGetXMLFromDom(vshControl *ctl,
virDomainPtr dom,
diff --git a/tools/virsh-util.h b/tools/virsh-util.h
index 7165755550..3ff6f16784 100644
--- a/tools/virsh-util.h
+++ b/tools/virsh-util.h
@@ -64,6 +64,11 @@ void
virshSecretFree(virSecretPtr secret);
G_DEFINE_AUTOPTR_CLEANUP_FUNC(virshSecret, virshSecretFree);
+typedef virStoragePool virshStoragePool;
+void
+virshStoragePoolFree(virStoragePoolPtr pool);
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(virshStoragePool, virshStoragePoolFree);
+
int
virshDomainState(vshControl *ctl,
virDomainPtr dom,
diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c
index 38bb62a48f..6e8f7721a3 100644
--- a/tools/virsh-volume.c
+++ b/tools/virsh-volume.c
@@ -83,7 +83,7 @@ virshCommandOptVolBy(vshControl *ctl, const vshCmd *cmd,
const char **name, unsigned int flags)
{
virStorageVolPtr vol = NULL;
- virStoragePoolPtr pool = NULL;
+ g_autoptr(virshStoragePool) pool = NULL;
const char *n = NULL, *p = NULL;
virshControl *priv = ctl->privData;
@@ -102,7 +102,6 @@ virshCommandOptVolBy(vshControl *ctl, const vshCmd *cmd,
if (virStoragePoolIsActive(pool) != 1) {
vshError(ctl, _("pool '%s' is not active"), p);
- virStoragePoolFree(pool);
return NULL;
}
}
@@ -145,7 +144,7 @@ virshCommandOptVolBy(vshControl *ctl, const vshCmd *cmd,
/* If the pool was specified, then make sure that the returned
* volume is from the given pool */
if (pool && vol) {
- virStoragePoolPtr volpool = NULL;
+ g_autoptr(virshStoragePool) volpool = NULL;
if ((volpool = virStoragePoolLookupByVolume(vol))) {
if (STRNEQ(virStoragePoolGetName(volpool),
@@ -157,13 +156,9 @@ virshCommandOptVolBy(vshControl *ctl, const vshCmd *cmd,
virStorageVolFree(vol);
vol = NULL;
}
- virStoragePoolFree(volpool);
}
}
- if (pool)
- virStoragePoolFree(pool);
-
return vol;
}
@@ -234,7 +229,7 @@ virshVolSize(const char *data, unsigned long long *val)
static bool
cmdVolCreateAs(vshControl *ctl, const vshCmd *cmd)
{
- virStoragePoolPtr pool;
+ g_autoptr(virshStoragePool) pool = NULL;
virStorageVolPtr vol = NULL;
g_autofree char *xml = NULL;
bool printXML = vshCommandOptBool(cmd, "print-xml");
@@ -373,7 +368,6 @@ cmdVolCreateAs(vshControl *ctl, const vshCmd *cmd)
cleanup:
if (vol)
virStorageVolFree(vol);
- virStoragePoolFree(pool);
return ret;
}
@@ -403,7 +397,7 @@ static const vshCmdOptDef opts_vol_create[] = {
static bool
cmdVolCreate(vshControl *ctl, const vshCmd *cmd)
{
- virStoragePoolPtr pool;
+ g_autoptr(virshStoragePool) pool = NULL;
virStorageVolPtr vol;
const char *from = NULL;
bool ret = false;
@@ -434,7 +428,6 @@ cmdVolCreate(vshControl *ctl, const vshCmd *cmd)
}
cleanup:
- virStoragePoolFree(pool);
return ret;
}
@@ -474,7 +467,7 @@ static const vshCmdOptDef opts_vol_create_from[] = {
static bool
cmdVolCreateFrom(vshControl *ctl, const vshCmd *cmd)
{
- virStoragePoolPtr pool = NULL;
+ g_autoptr(virshStoragePool) pool = NULL;
virStorageVolPtr newvol = NULL, inputvol = NULL;
const char *from = NULL;
bool ret = false;
@@ -513,8 +506,6 @@ cmdVolCreateFrom(vshControl *ctl, const vshCmd *cmd)
ret = true;
cleanup:
- if (pool)
- virStoragePoolFree(pool);
if (inputvol)
virStorageVolFree(inputvol);
if (newvol)
@@ -582,7 +573,7 @@ static const vshCmdOptDef opts_vol_clone[] = {
static bool
cmdVolClone(vshControl *ctl, const vshCmd *cmd)
{
- virStoragePoolPtr origpool = NULL;
+ g_autoptr(virshStoragePool) origpool = NULL;
virStorageVolPtr origvol = NULL, newvol = NULL;
const char *name = NULL;
g_autofree char *origxml = NULL;
@@ -637,8 +628,6 @@ cmdVolClone(vshControl *ctl, const vshCmd *cmd)
virStorageVolFree(origvol);
if (newvol)
virStorageVolFree(newvol);
- if (origpool)
- virStoragePoolFree(origpool);
return ret;
}
@@ -1395,7 +1384,7 @@ static bool
cmdVolList(vshControl *ctl, const vshCmd *cmd G_GNUC_UNUSED)
{
virStorageVolInfo volumeInfo;
- virStoragePoolPtr pool;
+ g_autoptr(virshStoragePool) pool = NULL;
const char *unit;
double val;
bool details = vshCommandOptBool(cmd, "details");
@@ -1521,7 +1510,6 @@ cmdVolList(vshControl *ctl, const vshCmd *cmd G_GNUC_UNUSED)
/* Cleanup remaining memory */
VIR_FREE(volInfoTexts);
- virStoragePoolFree(pool);
virshStorageVolListFree(list);
/* Return the desired value */
@@ -1585,7 +1573,7 @@ static const vshCmdOptDef opts_vol_pool[] = {
static bool
cmdVolPool(vshControl *ctl, const vshCmd *cmd)
{
- virStoragePoolPtr pool;
+ g_autoptr(virshStoragePool) pool = NULL;
virStorageVolPtr vol;
char uuid[VIR_UUID_STRING_BUFLEN];
@@ -1615,7 +1603,6 @@ cmdVolPool(vshControl *ctl, const vshCmd *cmd)
/* Cleanup */
virStorageVolFree(vol);
- virStoragePoolFree(pool);
return true;
}
--
2.32.0
12:07 a.m.
New subject: [PATCH 4/8] virsh: Add wrapper for virStorageVolFree
Similarly to virshDomainFree add a wrapper for the snapshot object
freeing function.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
build-aux/syntax-check.mk | 2 +-
tools/virsh-completer-volume.c | 4 +-
tools/virsh-domain.c | 3 +-
tools/virsh-util.c | 11 ++++++
tools/virsh-util.h | 5 +++
tools/virsh-volume.c | 72 ++++++++++------------------------
6 files changed, 40 insertions(+), 57 deletions(-)
diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk
index 111d2109e8..2bdbd14c80 100644
--- a/build-aux/syntax-check.mk
+++ b/build-aux/syntax-check.mk
@@ -868,7 +868,7 @@ sc_gettext_init:
$(_sc_search_regexp)
sc_prohibit_obj_free_apis_in_virsh:
- @prohibit='\bvir(Domain|DomainSnapshot|Interface|Secret|StoragePool)Free\b' \
+ @prohibit='\bvir(Domain|DomainSnapshot|Interface|Secret|StoragePool|StorageVol)Free\b'
\
in_vc_files='virsh.*\.[ch]$$' \
exclude='sc_prohibit_obj_free_apis_in_virsh' \
halt='avoid using public virXXXFree in virsh, use virsh-prefixed wrappers
instead' \
diff --git a/tools/virsh-completer-volume.c b/tools/virsh-completer-volume.c
index 1d83643c69..ac3c472177 100644
--- a/tools/virsh-completer-volume.c
+++ b/tools/virsh-completer-volume.c
@@ -65,7 +65,7 @@ virshStorageVolNameCompleter(vshControl *ctl,
cleanup:
for (i = 0; i < nvols; i++)
- virStorageVolFree(vols[i]);
+ virshStorageVolFree(vols[i]);
g_free(vols);
return ret;
}
@@ -104,7 +104,7 @@ virshStorageVolKeyCompleter(vshControl *ctl,
const char *key = virStorageVolGetKey(vols[j]);
tmp[nvols] = g_strdup(key);
nvols++;
- virStorageVolFree(vols[j]);
+ virshStorageVolFree(vols[j]);
}
g_free(vols);
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 461a5e19f6..b1943b3875 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -3942,8 +3942,7 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd)
for (i = 0; i < nvols; i++) {
VIR_FREE(vols[i].source);
VIR_FREE(vols[i].target);
- if (vols[i].vol)
- virStorageVolFree(vols[i].vol);
+ virshStorageVolFree(vols[i].vol);
}
VIR_FREE(vols);
diff --git a/tools/virsh-util.c b/tools/virsh-util.c
index d537501387..c680c5b3d4 100644
--- a/tools/virsh-util.c
+++ b/tools/virsh-util.c
@@ -318,6 +318,17 @@ virshStoragePoolFree(virStoragePoolPtr pool)
}
+void
+virshStorageVolFree(virStorageVolPtr vol)
+{
+ if (!vol)
+ return;
+
+ vshSaveLibvirtHelperError();
+ virStorageVolFree(vol); /* sc_prohibit_obj_free_apis_in_virsh */
+}
+
+
int
virshDomainGetXMLFromDom(vshControl *ctl,
virDomainPtr dom,
diff --git a/tools/virsh-util.h b/tools/virsh-util.h
index 3ff6f16784..ce3462a865 100644
--- a/tools/virsh-util.h
+++ b/tools/virsh-util.h
@@ -69,6 +69,11 @@ void
virshStoragePoolFree(virStoragePoolPtr pool);
G_DEFINE_AUTOPTR_CLEANUP_FUNC(virshStoragePool, virshStoragePoolFree);
+typedef virStorageVol virshStorageVol;
+void
+virshStorageVolFree(virStorageVolPtr vol);
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(virshStorageVol, virshStorageVolFree);
+
int
virshDomainState(vshControl *ctl,
virDomainPtr dom,
diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c
index 6e8f7721a3..b896ebbbf9 100644
--- a/tools/virsh-volume.c
+++ b/tools/virsh-volume.c
@@ -153,8 +153,7 @@ virshCommandOptVolBy(vshControl *ctl, const vshCmd *cmd,
vshError(ctl,
_("Requested volume '%s' is not in pool
'%s'"),
n, virStoragePoolGetName(pool));
- virStorageVolFree(vol);
- vol = NULL;
+ g_clear_pointer(&vol, virshStorageVolFree);
}
}
}
@@ -230,7 +229,7 @@ static bool
cmdVolCreateAs(vshControl *ctl, const vshCmd *cmd)
{
g_autoptr(virshStoragePool) pool = NULL;
- virStorageVolPtr vol = NULL;
+ g_autoptr(virshStorageVol) vol = NULL;
g_autofree char *xml = NULL;
bool printXML = vshCommandOptBool(cmd, "print-xml");
const char *name, *capacityStr = NULL, *allocationStr = NULL, *format = NULL;
@@ -287,7 +286,7 @@ cmdVolCreateAs(vshControl *ctl, const vshCmd *cmd)
/* Convert the snapshot parameters into backingStore XML */
if (snapshotStrVol) {
- virStorageVolPtr snapVol;
+ g_autoptr(virshStorageVol) snapVol = NULL;
g_autofree char *snapshotStrVolPath = NULL;
/* Lookup snapshot backing volume. Try the backing-vol
* parameter as a name */
@@ -330,7 +329,6 @@ cmdVolCreateAs(vshControl *ctl, const vshCmd *cmd)
}
if ((snapshotStrVolPath = virStorageVolGetPath(snapVol)) == NULL) {
- virStorageVolFree(snapVol);
goto cleanup;
}
@@ -343,9 +341,6 @@ cmdVolCreateAs(vshControl *ctl, const vshCmd *cmd)
snapshotStrFormat);
virBufferAdjustIndent(&buf, -2);
virBufferAddLit(&buf, "</backingStore>\n");
-
- /* Cleanup snapshot allocations */
- virStorageVolFree(snapVol);
}
virBufferAdjustIndent(&buf, -2);
@@ -366,8 +361,6 @@ cmdVolCreateAs(vshControl *ctl, const vshCmd *cmd)
ret = true;
cleanup:
- if (vol)
- virStorageVolFree(vol);
return ret;
}
@@ -398,7 +391,7 @@ static bool
cmdVolCreate(vshControl *ctl, const vshCmd *cmd)
{
g_autoptr(virshStoragePool) pool = NULL;
- virStorageVolPtr vol;
+ g_autoptr(virshStorageVol) vol = NULL;
const char *from = NULL;
bool ret = false;
unsigned int flags = 0;
@@ -421,7 +414,6 @@ cmdVolCreate(vshControl *ctl, const vshCmd *cmd)
if ((vol = virStorageVolCreateXML(pool, buffer, flags))) {
vshPrintExtra(ctl, _("Vol %s created from %s\n"),
virStorageVolGetName(vol), from);
- virStorageVolFree(vol);
ret = true;
} else {
vshError(ctl, _("Failed to create vol from %s"), from);
@@ -468,7 +460,8 @@ static bool
cmdVolCreateFrom(vshControl *ctl, const vshCmd *cmd)
{
g_autoptr(virshStoragePool) pool = NULL;
- virStorageVolPtr newvol = NULL, inputvol = NULL;
+ g_autoptr(virshStorageVol) newvol = NULL;
+ g_autoptr(virshStorageVol) inputvol = NULL;
const char *from = NULL;
bool ret = false;
g_autofree char *buffer = NULL;
@@ -506,10 +499,6 @@ cmdVolCreateFrom(vshControl *ctl, const vshCmd *cmd)
ret = true;
cleanup:
- if (inputvol)
- virStorageVolFree(inputvol);
- if (newvol)
- virStorageVolFree(newvol);
return ret;
}
@@ -574,7 +563,8 @@ static bool
cmdVolClone(vshControl *ctl, const vshCmd *cmd)
{
g_autoptr(virshStoragePool) origpool = NULL;
- virStorageVolPtr origvol = NULL, newvol = NULL;
+ g_autoptr(virshStorageVol) origvol = NULL;
+ g_autoptr(virshStorageVol) newvol = NULL;
const char *name = NULL;
g_autofree char *origxml = NULL;
xmlChar *newxml = NULL;
@@ -624,10 +614,6 @@ cmdVolClone(vshControl *ctl, const vshCmd *cmd)
cleanup:
xmlFree(newxml);
- if (origvol)
- virStorageVolFree(origvol);
- if (newvol)
- virStorageVolFree(newvol);
return ret;
}
@@ -667,7 +653,7 @@ static bool
cmdVolUpload(vshControl *ctl, const vshCmd *cmd)
{
const char *file = NULL;
- virStorageVolPtr vol = NULL;
+ g_autoptr(virshStorageVol) vol = NULL;
bool ret = false;
int fd = -1;
virStreamPtr st = NULL;
@@ -745,8 +731,6 @@ cmdVolUpload(vshControl *ctl, const vshCmd *cmd)
ret = true;
cleanup:
- if (vol)
- virStorageVolFree(vol);
if (st)
virStreamFree(st);
VIR_FORCE_CLOSE(fd);
@@ -789,7 +773,7 @@ static bool
cmdVolDownload(vshControl *ctl, const vshCmd *cmd)
{
const char *file = NULL;
- virStorageVolPtr vol = NULL;
+ g_autoptr(virshStorageVol) vol = NULL;
bool ret = false;
int fd = -1;
virStreamPtr st = NULL;
@@ -867,8 +851,6 @@ cmdVolDownload(vshControl *ctl, const vshCmd *cmd)
VIR_FORCE_CLOSE(fd);
if (!ret && created)
unlink(file);
- if (vol)
- virStorageVolFree(vol);
if (st)
virStreamFree(st);
return ret;
@@ -901,7 +883,7 @@ static const vshCmdOptDef opts_vol_delete[] = {
static bool
cmdVolDelete(vshControl *ctl, const vshCmd *cmd)
{
- virStorageVolPtr vol;
+ g_autoptr(virshStorageVol) vol = NULL;
bool ret = true;
const char *name;
bool delete_snapshots = vshCommandOptBool(cmd, "delete-snapshots");
@@ -920,7 +902,6 @@ cmdVolDelete(vshControl *ctl, const vshCmd *cmd)
ret = false;
}
- virStorageVolFree(vol);
return ret;
}
@@ -956,7 +937,7 @@ VIR_ENUM_IMPL(virshStorageVolWipeAlgorithm,
static bool
cmdVolWipe(vshControl *ctl, const vshCmd *cmd)
{
- virStorageVolPtr vol;
+ g_autoptr(virshStorageVol) vol = NULL;
bool ret = false;
const char *name;
const char *algorithm_str = NULL;
@@ -989,7 +970,6 @@ cmdVolWipe(vshControl *ctl, const vshCmd *cmd)
vshPrintExtra(ctl, _("Vol %s wiped\n"), name);
ret = true;
out:
- virStorageVolFree(vol);
return ret;
}
@@ -1043,7 +1023,7 @@ static bool
cmdVolInfo(vshControl *ctl, const vshCmd *cmd)
{
virStorageVolInfo info;
- virStorageVolPtr vol;
+ g_autoptr(virshStorageVol) vol = NULL;
bool bytes = vshCommandOptBool(cmd, "bytes");
bool physical = vshCommandOptBool(cmd, "physical");
int rc;
@@ -1063,7 +1043,6 @@ cmdVolInfo(vshControl *ctl, const vshCmd *cmd)
rc = virStorageVolGetInfo(vol, &info);
if (rc < 0) {
- virStorageVolFree(vol);
return false;
}
@@ -1090,7 +1069,6 @@ cmdVolInfo(vshControl *ctl, const vshCmd *cmd)
vshPrint(ctl, "%-15s %2.2lf %s\n", _("Allocation:"), val,
unit);
}
- virStorageVolFree(vol);
return true;
}
@@ -1136,7 +1114,7 @@ static const vshCmdOptDef opts_vol_resize[] = {
static bool
cmdVolResize(vshControl *ctl, const vshCmd *cmd)
{
- virStorageVolPtr vol;
+ g_autoptr(virshStorageVol) vol = NULL;
const char *capacityStr = NULL;
unsigned long long capacity = 0;
unsigned int flags = 0;
@@ -1190,7 +1168,6 @@ cmdVolResize(vshControl *ctl, const vshCmd *cmd)
}
cleanup:
- virStorageVolFree(vol);
return ret;
}
@@ -1216,7 +1193,7 @@ static const vshCmdOptDef opts_vol_dumpxml[] = {
static bool
cmdVolDumpXML(vshControl *ctl, const vshCmd *cmd)
{
- virStorageVolPtr vol;
+ g_autoptr(virshStorageVol) vol = NULL;
bool ret = true;
char *dump;
@@ -1231,7 +1208,6 @@ cmdVolDumpXML(vshControl *ctl, const vshCmd *cmd)
ret = false;
}
- virStorageVolFree(vol);
return ret;
}
@@ -1263,8 +1239,7 @@ virshStorageVolListFree(struct virshStorageVolList *list)
if (list && list->vols) {
for (i = 0; i < list->nvols; i++) {
- if (list->vols[i])
- virStorageVolFree(list->vols[i]);
+ virshStorageVolFree(list->vols[i]);
}
g_free(list->vols);
}
@@ -1537,14 +1512,13 @@ static const vshCmdOptDef opts_vol_name[] = {
static bool
cmdVolName(vshControl *ctl, const vshCmd *cmd)
{
- virStorageVolPtr vol;
+ g_autoptr(virshStorageVol) vol = NULL;
if (!(vol = virshCommandOptVolBy(ctl, cmd, "vol", NULL, NULL,
VIRSH_BYUUID)))
return false;
vshPrint(ctl, "%s\n", virStorageVolGetName(vol));
- virStorageVolFree(vol);
return true;
}
@@ -1574,7 +1548,7 @@ static bool
cmdVolPool(vshControl *ctl, const vshCmd *cmd)
{
g_autoptr(virshStoragePool) pool = NULL;
- virStorageVolPtr vol;
+ g_autoptr(virshStorageVol) vol = NULL;
char uuid[VIR_UUID_STRING_BUFLEN];
/* Use the supplied string to locate the volume */
@@ -1587,7 +1561,6 @@ cmdVolPool(vshControl *ctl, const vshCmd *cmd)
pool = virStoragePoolLookupByVolume(vol);
if (pool == NULL) {
vshError(ctl, "%s", _("failed to get parent pool"));
- virStorageVolFree(vol);
return false;
}
@@ -1601,8 +1574,6 @@ cmdVolPool(vshControl *ctl, const vshCmd *cmd)
vshPrint(ctl, "%s\n", virStoragePoolGetName(pool));
}
- /* Cleanup */
- virStorageVolFree(vol);
return true;
}
@@ -1628,13 +1599,12 @@ static const vshCmdOptDef opts_vol_key[] = {
static bool
cmdVolKey(vshControl *ctl, const vshCmd *cmd)
{
- virStorageVolPtr vol;
+ g_autoptr(virshStorageVol) vol = NULL;
if (!(vol = virshCommandOptVol(ctl, cmd, "vol", "pool", NULL)))
return false;
vshPrint(ctl, "%s\n", virStorageVolGetKey(vol));
- virStorageVolFree(vol);
return true;
}
@@ -1660,19 +1630,17 @@ static const vshCmdOptDef opts_vol_path[] = {
static bool
cmdVolPath(vshControl *ctl, const vshCmd *cmd)
{
- virStorageVolPtr vol;
+ g_autoptr(virshStorageVol) vol = NULL;
g_autofree char *StorageVolPath = NULL;
if (!(vol = virshCommandOptVol(ctl, cmd, "vol", "pool", NULL)))
return false;
if ((StorageVolPath = virStorageVolGetPath(vol)) == NULL) {
- virStorageVolFree(vol);
return false;
}
vshPrint(ctl, "%s\n", StorageVolPath);
- virStorageVolFree(vol);
return true;
}
--
2.32.0
12:07 a.m.
New subject: [PATCH 5/8] virsh: Add wrapper for virNetworkFree
Similarly to virshDomainFree add a wrapper for the snapshot object
freeing function.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
build-aux/syntax-check.mk | 2 +-
tools/virsh-completer-network.c | 8 ++--
tools/virsh-network.c | 75 ++++++++++-----------------------
tools/virsh-util.c | 11 +++++
tools/virsh-util.h | 5 +++
5 files changed, 43 insertions(+), 58 deletions(-)
diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk
index 2bdbd14c80..0c0d844d6c 100644
--- a/build-aux/syntax-check.mk
+++ b/build-aux/syntax-check.mk
@@ -868,7 +868,7 @@ sc_gettext_init:
$(_sc_search_regexp)
sc_prohibit_obj_free_apis_in_virsh:
- @prohibit='\bvir(Domain|DomainSnapshot|Interface|Secret|StoragePool|StorageVol)Free\b'
\
+ @prohibit='\bvir(Domain|DomainSnapshot|Interface|Network|Secret|StoragePool|StorageVol)Free\b'
\
in_vc_files='virsh.*\.[ch]$$' \
exclude='sc_prohibit_obj_free_apis_in_virsh' \
halt='avoid using public virXXXFree in virsh, use virsh-prefixed wrappers
instead' \
diff --git a/tools/virsh-completer-network.c b/tools/virsh-completer-network.c
index f9154f23a4..d498f59cb8 100644
--- a/tools/virsh-completer-network.c
+++ b/tools/virsh-completer-network.c
@@ -21,6 +21,7 @@
#include <config.h>
#include "virsh-completer-network.h"
+#include "virsh-util.h"
#include "viralloc.h"
#include "virsh-network.h"
#include "virsh.h"
@@ -60,7 +61,7 @@ virshNetworkNameCompleter(vshControl *ctl,
ret = g_steal_pointer(&tmp);
for (i = 0; i < nnets; i++)
- virNetworkFree(nets[i]);
+ virshNetworkFree(nets[i]);
g_free(nets);
return ret;
}
@@ -170,7 +171,7 @@ virshNetworkUUIDCompleter(vshControl *ctl,
cleanup:
for (i = 0; i < nnets; i++)
- virNetworkFree(nets[i]);
+ virshNetworkFree(nets[i]);
g_free(nets);
return ret;
}
@@ -183,7 +184,7 @@ virshNetworkDhcpMacCompleter(vshControl *ctl,
{
virshControl *priv = ctl->privData;
virNetworkDHCPLeasePtr *leases = NULL;
- virNetworkPtr network = NULL;
+ g_autoptr(virshNetwork) network = NULL;
int nleases;
size_t i = 0;
char **ret = NULL;
@@ -215,6 +216,5 @@ virshNetworkDhcpMacCompleter(vshControl *ctl,
virNetworkDHCPLeaseFree(leases[i]);
VIR_FREE(leases);
}
- virNetworkFree(network);
return ret;
}
diff --git a/tools/virsh-network.c b/tools/virsh-network.c
index 1442210278..02b22bf912 100644
--- a/tools/virsh-network.c
+++ b/tools/virsh-network.c
@@ -20,6 +20,7 @@
#include <config.h>
#include "virsh-network.h"
+#include "virsh-util.h"
#include "internal.h"
#include "viralloc.h"
@@ -155,7 +156,7 @@ static const vshCmdOptDef opts_network_autostart[] = {
static bool
cmdNetworkAutostart(vshControl *ctl, const vshCmd *cmd)
{
- virNetworkPtr network;
+ g_autoptr(virshNetwork) network = NULL;
const char *name;
int autostart;
@@ -169,7 +170,6 @@ cmdNetworkAutostart(vshControl *ctl, const vshCmd *cmd)
vshError(ctl, _("failed to mark network %s as autostarted"),
name);
else
vshError(ctl, _("failed to unmark network %s as autostarted"),
name);
- virNetworkFree(network);
return false;
}
@@ -178,7 +178,6 @@ cmdNetworkAutostart(vshControl *ctl, const vshCmd *cmd)
else
vshPrintExtra(ctl, _("Network %s unmarked as autostarted\n"), name);
- virNetworkFree(network);
return true;
}
@@ -207,7 +206,7 @@ static const vshCmdOptDef opts_network_create[] = {
static bool
cmdNetworkCreate(vshControl *ctl, const vshCmd *cmd)
{
- virNetworkPtr network;
+ g_autoptr(virshNetwork) network = NULL;
const char *from = NULL;
g_autofree char *buffer = NULL;
unsigned int flags = 0;
@@ -234,7 +233,6 @@ cmdNetworkCreate(vshControl *ctl, const vshCmd *cmd)
vshPrintExtra(ctl, _("Network %s created from %s\n"),
virNetworkGetName(network), from);
- virNetworkFree(network);
return true;
}
@@ -264,7 +262,7 @@ static const vshCmdOptDef opts_network_define[] = {
static bool
cmdNetworkDefine(vshControl *ctl, const vshCmd *cmd)
{
- virNetworkPtr network;
+ g_autoptr(virshNetwork) network = NULL;
const char *from = NULL;
g_autofree char *buffer = NULL;
unsigned int flags = 0;
@@ -291,7 +289,6 @@ cmdNetworkDefine(vshControl *ctl, const vshCmd *cmd)
vshPrintExtra(ctl, _("Network %s defined from %s\n"),
virNetworkGetName(network), from);
- virNetworkFree(network);
return true;
}
@@ -316,7 +313,7 @@ static const vshCmdOptDef opts_network_destroy[] = {
static bool
cmdNetworkDestroy(vshControl *ctl, const vshCmd *cmd)
{
- virNetworkPtr network;
+ g_autoptr(virshNetwork) network = NULL;
bool ret = true;
const char *name;
@@ -330,7 +327,6 @@ cmdNetworkDestroy(vshControl *ctl, const vshCmd *cmd)
ret = false;
}
- virNetworkFree(network);
return ret;
}
@@ -359,7 +355,7 @@ static const vshCmdOptDef opts_network_dumpxml[] = {
static bool
cmdNetworkDumpXML(vshControl *ctl, const vshCmd *cmd)
{
- virNetworkPtr network;
+ g_autoptr(virshNetwork) network = NULL;
g_autofree char *dump = NULL;
unsigned int flags = 0;
@@ -370,12 +366,10 @@ cmdNetworkDumpXML(vshControl *ctl, const vshCmd *cmd)
flags |= VIR_NETWORK_XML_INACTIVE;
if (!(dump = virNetworkGetXMLDesc(network, flags))) {
- virNetworkFree(network);
return false;
}
vshPrint(ctl, "%s", dump);
- virNetworkFree(network);
return true;
}
@@ -400,7 +394,7 @@ static const vshCmdOptDef opts_network_info[] = {
static bool
cmdNetworkInfo(vshControl *ctl, const vshCmd *cmd)
{
- virNetworkPtr network;
+ g_autoptr(virshNetwork) network = NULL;
char uuid[VIR_UUID_STRING_BUFLEN];
int autostart;
int persistent = -1;
@@ -435,7 +429,6 @@ cmdNetworkInfo(vshControl *ctl, const vshCmd *cmd)
vshPrint(ctl, "%-15s %s\n", _("Bridge:"), bridge);
VIR_FREE(bridge);
- virNetworkFree(network);
return true;
}
@@ -467,8 +460,7 @@ virshNetworkListFree(struct virshNetworkList *list)
if (list && list->nets) {
for (i = 0; i < list->nnets; i++) {
- if (list->nets[i])
- virNetworkFree(list->nets[i]);
+ virshNetworkFree(list->nets[i]);
}
g_free(list->nets);
}
@@ -626,8 +618,7 @@ virshNetworkListCollect(vshControl *ctl,
remove_entry:
/* the pool has to be removed as it failed one of the filters */
- virNetworkFree(list->nets[i]);
- list->nets[i] = NULL;
+ g_clear_pointer(&list->nets[i], virshNetworkFree);
deleted++;
}
@@ -825,14 +816,13 @@ static const vshCmdOptDef opts_network_name[] = {
static bool
cmdNetworkName(vshControl *ctl, const vshCmd *cmd)
{
- virNetworkPtr network;
+ g_autoptr(virshNetwork) network = NULL;
if (!(network = virshCommandOptNetworkBy(ctl, cmd, NULL,
VIRSH_BYUUID)))
return false;
vshPrint(ctl, "%s\n", virNetworkGetName(network));
- virNetworkFree(network);
return true;
}
@@ -857,7 +847,7 @@ static const vshCmdOptDef opts_network_start[] = {
static bool
cmdNetworkStart(vshControl *ctl, const vshCmd *cmd)
{
- virNetworkPtr network;
+ g_autoptr(virshNetwork) network = NULL;
bool ret = true;
const char *name = NULL;
@@ -870,7 +860,6 @@ cmdNetworkStart(vshControl *ctl, const vshCmd *cmd)
vshError(ctl, _("Failed to start network %s"), name);
ret = false;
}
- virNetworkFree(network);
return ret;
}
@@ -895,7 +884,7 @@ static const vshCmdOptDef opts_network_undefine[] = {
static bool
cmdNetworkUndefine(vshControl *ctl, const vshCmd *cmd)
{
- virNetworkPtr network;
+ g_autoptr(virshNetwork) network = NULL;
bool ret = true;
const char *name;
@@ -909,7 +898,6 @@ cmdNetworkUndefine(vshControl *ctl, const vshCmd *cmd)
ret = false;
}
- virNetworkFree(network);
return ret;
}
@@ -972,7 +960,7 @@ static bool
cmdNetworkUpdate(vshControl *ctl, const vshCmd *cmd)
{
bool ret = false;
- virNetworkPtr network;
+ g_autoptr(virshNetwork) network = NULL;
const char *commandStr = NULL;
const char *sectionStr = NULL;
int command, section, parentIndex = -1;
@@ -1071,7 +1059,6 @@ cmdNetworkUpdate(vshControl *ctl, const vshCmd *cmd)
ret = true;
cleanup:
vshReportError(ctl);
- virNetworkFree(network);
return ret;
}
@@ -1096,7 +1083,7 @@ static const vshCmdOptDef opts_network_uuid[] = {
static bool
cmdNetworkUuid(vshControl *ctl, const vshCmd *cmd)
{
- virNetworkPtr network;
+ g_autoptr(virshNetwork) network = NULL;
char uuid[VIR_UUID_STRING_BUFLEN];
if (!(network = virshCommandOptNetworkBy(ctl, cmd, NULL,
@@ -1108,7 +1095,6 @@ cmdNetworkUuid(vshControl *ctl, const vshCmd *cmd)
else
vshError(ctl, "%s", _("failed to get network UUID"));
- virNetworkFree(network);
return true;
}
@@ -1150,8 +1136,8 @@ static bool
cmdNetworkEdit(vshControl *ctl, const vshCmd *cmd)
{
bool ret = false;
- virNetworkPtr network = NULL;
- virNetworkPtr network_edited = NULL;
+ g_autoptr(virshNetwork) network = NULL;
+ g_autoptr(virshNetwork) network_edited = NULL;
virshControl *priv = ctl->privData;
network = virshCommandOptNetwork(ctl, cmd, NULL);
@@ -1176,11 +1162,6 @@ cmdNetworkEdit(vshControl *ctl, const vshCmd *cmd)
ret = true;
cleanup:
- if (network)
- virNetworkFree(network);
- if (network_edited)
- virNetworkFree(network_edited);
-
return ret;
}
@@ -1293,7 +1274,7 @@ static const vshCmdOptDef opts_network_event[] = {
static bool
cmdNetworkEvent(vshControl *ctl, const vshCmd *cmd)
{
- virNetworkPtr net = NULL;
+ g_autoptr(virshNetwork) net = NULL;
bool ret = false;
int eventId = -1;
int timeout = 0;
@@ -1362,8 +1343,6 @@ cmdNetworkEvent(vshControl *ctl, const vshCmd *cmd)
if (eventId >= 0 &&
virConnectNetworkEventDeregisterAny(priv->conn, eventId) < 0)
ret = false;
- if (net)
- virNetworkFree(net);
return ret;
}
@@ -1417,7 +1396,7 @@ cmdNetworkDHCPLeases(vshControl *ctl, const vshCmd *cmd)
bool ret = false;
size_t i;
unsigned int flags = 0;
- virNetworkPtr network = NULL;
+ g_autoptr(virshNetwork) network = NULL;
g_autoptr(vshTable) table = NULL;
if (vshCommandOptStringReq(ctl, cmd, "mac", &mac) < 0)
@@ -1477,7 +1456,6 @@ cmdNetworkDHCPLeases(vshControl *ctl, const vshCmd *cmd)
virNetworkDHCPLeaseFree(leases[i]);
VIR_FREE(leases);
}
- virNetworkFree(network);
return ret;
}
@@ -1511,7 +1489,7 @@ cmdNetworkPortCreate(vshControl *ctl, const vshCmd *cmd)
const char *from = NULL;
bool ret = false;
char *buffer = NULL;
- virNetworkPtr network = NULL;
+ g_autoptr(virshNetwork) network = NULL;
unsigned int flags = 0;
network = virshCommandOptNetwork(ctl, cmd, NULL);
@@ -1546,8 +1524,6 @@ cmdNetworkPortCreate(vshControl *ctl, const vshCmd *cmd)
VIR_FREE(buffer);
if (port)
virNetworkPortFree(port);
- if (network)
- virNetworkFree(network);
return ret;
}
@@ -1573,7 +1549,7 @@ static const vshCmdOptDef opts_network_port_dumpxml[] = {
static bool
cmdNetworkPortDumpXML(vshControl *ctl, const vshCmd *cmd)
{
- virNetworkPtr network;
+ g_autoptr(virshNetwork) network = NULL;
virNetworkPortPtr port = NULL;
bool ret = true;
g_autofree char *dump = NULL;
@@ -1596,8 +1572,6 @@ cmdNetworkPortDumpXML(vshControl *ctl, const vshCmd *cmd)
cleanup:
if (port)
virNetworkPortFree(port);
- if (network)
- virNetworkFree(network);
return ret;
}
@@ -1624,7 +1598,7 @@ static const vshCmdOptDef opts_network_port_delete[] = {
static bool
cmdNetworkPortDelete(vshControl *ctl, const vshCmd *cmd)
{
- virNetworkPtr network = NULL;
+ g_autoptr(virshNetwork) network = NULL;
virNetworkPortPtr port = NULL;
bool ret = true;
char uuidstr[VIR_UUID_STRING_BUFLEN];
@@ -1649,8 +1623,6 @@ cmdNetworkPortDelete(vshControl *ctl, const vshCmd *cmd)
cleanup:
if (port)
virNetworkPortFree(port);
- if (network)
- virNetworkFree(network);
return ret;
}
@@ -1703,7 +1675,7 @@ virshNetworkPortListCollect(vshControl *ctl,
{
struct virshNetworkPortList *list = g_new0(struct virshNetworkPortList, 1);
int ret;
- virNetworkPtr network = NULL;
+ g_autoptr(virshNetwork) network = NULL;
bool success = false;
if (!(network = virshCommandOptNetwork(ctl, cmd, NULL)))
@@ -1729,9 +1701,6 @@ virshNetworkPortListCollect(vshControl *ctl,
list = NULL;
}
- if (network)
- virNetworkFree(network);
-
return list;
}
diff --git a/tools/virsh-util.c b/tools/virsh-util.c
index c680c5b3d4..f7b649983e 100644
--- a/tools/virsh-util.c
+++ b/tools/virsh-util.c
@@ -296,6 +296,17 @@ virshInterfaceFree(virInterfacePtr iface)
}
+void
+virshNetworkFree(virNetworkPtr network)
+{
+ if (!network)
+ return;
+
+ vshSaveLibvirtHelperError();
+ virNetworkFree(network); /* sc_prohibit_obj_free_apis_in_virsh */
+}
+
+
void
virshSecretFree(virSecretPtr secret)
{
diff --git a/tools/virsh-util.h b/tools/virsh-util.h
index ce3462a865..e8df0a6618 100644
--- a/tools/virsh-util.h
+++ b/tools/virsh-util.h
@@ -59,6 +59,11 @@ void
virshInterfaceFree(virInterfacePtr iface);
G_DEFINE_AUTOPTR_CLEANUP_FUNC(virshInterface, virshInterfaceFree);
+typedef virNetwork virshNetwork;
+void
+virshNetworkFree(virNetworkPtr network);
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(virshNetwork, virshNetworkFree);
+
typedef virSecret virshSecret;
void
virshSecretFree(virSecretPtr secret);
--
2.32.0
12:07 a.m.
New subject: [PATCH 6/8] virsh: Add wrapper for virNodeDeviceFree
Similarly to virshDomainFree add a wrapper for the snapshot object
freeing function.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
build-aux/syntax-check.mk | 2 +-
tools/virsh-completer-nodedev.c | 3 +-
tools/virsh-nodedev.c | 49 ++++++++++-----------------------
tools/virsh-util.c | 11 ++++++++
tools/virsh-util.h | 5 ++++
5 files changed, 34 insertions(+), 36 deletions(-)
diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk
index 0c0d844d6c..6c230826bd 100644
--- a/build-aux/syntax-check.mk
+++ b/build-aux/syntax-check.mk
@@ -868,7 +868,7 @@ sc_gettext_init:
$(_sc_search_regexp)
sc_prohibit_obj_free_apis_in_virsh:
- @prohibit='\bvir(Domain|DomainSnapshot|Interface|Network|Secret|StoragePool|StorageVol)Free\b'
\
+ @prohibit='\bvir(Domain|DomainSnapshot|Interface|Network|NodeDevice|Secret|StoragePool|StorageVol)Free\b'
\
in_vc_files='virsh.*\.[ch]$$' \
exclude='sc_prohibit_obj_free_apis_in_virsh' \
halt='avoid using public virXXXFree in virsh, use virsh-prefixed wrappers
instead' \
diff --git a/tools/virsh-completer-nodedev.c b/tools/virsh-completer-nodedev.c
index d595b687fd..d10bf2b78c 100644
--- a/tools/virsh-completer-nodedev.c
+++ b/tools/virsh-completer-nodedev.c
@@ -21,6 +21,7 @@
#include <config.h>
#include "virsh-completer-nodedev.h"
+#include "virsh-util.h"
#include "conf/node_device_conf.h"
#include "viralloc.h"
#include "virsh-nodedev.h"
@@ -58,7 +59,7 @@ virshNodeDeviceNameCompleter(vshControl *ctl,
ret = g_steal_pointer(&tmp);
for (i = 0; i < ndevs; i++)
- virNodeDeviceFree(devs[i]);
+ virshNodeDeviceFree(devs[i]);
g_free(devs);
return ret;
}
diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c
index f72359121f..c989a77ad2 100644
--- a/tools/virsh-nodedev.c
+++ b/tools/virsh-nodedev.c
@@ -20,6 +20,7 @@
#include <config.h>
#include "virsh-nodedev.h"
+#include "virsh-util.h"
#include "internal.h"
#include "viralloc.h"
@@ -55,7 +56,7 @@ static const vshCmdOptDef opts_node_device_create[] = {
static bool
cmdNodeDeviceCreate(vshControl *ctl, const vshCmd *cmd)
{
- virNodeDevicePtr dev = NULL;
+ g_autoptr(virshNodeDevice) dev = NULL;
const char *from = NULL;
g_autofree char *buffer = NULL;
virshControl *priv = ctl->privData;
@@ -73,7 +74,6 @@ cmdNodeDeviceCreate(vshControl *ctl, const vshCmd *cmd)
vshPrintExtra(ctl, _("Node device %s created from %s\n"),
virNodeDeviceGetName(dev), from);
- virNodeDeviceFree(dev);
return true;
}
@@ -140,7 +140,7 @@ vshFindNodeDevice(vshControl *ctl, const char *value)
static bool
cmdNodeDeviceDestroy(vshControl *ctl, const vshCmd *cmd)
{
- virNodeDevice *dev = NULL;
+ g_autoptr(virshNodeDevice) dev = NULL;
bool ret = false;
const char *device_value = NULL;
@@ -160,8 +160,6 @@ cmdNodeDeviceDestroy(vshControl *ctl, const vshCmd *cmd)
ret = true;
cleanup:
- if (dev)
- virNodeDeviceFree(dev);
return ret;
}
@@ -207,8 +205,7 @@ virshNodeDeviceListFree(struct virshNodeDeviceList *list)
if (list && list->devices) {
for (i = 0; i < list->ndevices; i++) {
- if (list->devices[i])
- virNodeDeviceFree(list->devices[i]);
+ virshNodeDeviceFree(list->devices[i]);
}
g_free(list->devices);
}
@@ -327,8 +324,7 @@ virshNodeDeviceListCollect(vshControl *ctl,
remove_entry:
/* the device has to be removed as it failed one of the filters */
- virNodeDeviceFree(list->devices[i]);
- list->devices[i] = NULL;
+ g_clear_pointer(&list->devices[i], virshNodeDeviceFree);
deleted++;
}
@@ -576,7 +572,7 @@ static const vshCmdOptDef opts_node_device_dumpxml[] = {
static bool
cmdNodeDeviceDumpXML(vshControl *ctl, const vshCmd *cmd)
{
- virNodeDevicePtr device = NULL;
+ g_autoptr(virshNodeDevice) device = NULL;
g_autofree char *xml = NULL;
const char *device_value = NULL;
bool ret = false;
@@ -596,8 +592,6 @@ cmdNodeDeviceDumpXML(vshControl *ctl, const vshCmd *cmd)
ret = true;
cleanup:
- if (device)
- virNodeDeviceFree(device);
return ret;
}
@@ -634,7 +628,7 @@ cmdNodeDeviceDetach(vshControl *ctl, const vshCmd *cmd)
{
const char *name = NULL;
const char *driverName = NULL;
- virNodeDevicePtr device;
+ g_autoptr(virshNodeDevice) device = NULL;
bool ret = true;
virshControl *priv = ctl->privData;
@@ -664,7 +658,6 @@ cmdNodeDeviceDetach(vshControl *ctl, const vshCmd *cmd)
else
vshError(ctl, _("Failed to detach device %s"), name);
- virNodeDeviceFree(device);
return ret;
}
@@ -696,7 +689,7 @@ static bool
cmdNodeDeviceReAttach(vshControl *ctl, const vshCmd *cmd)
{
const char *name = NULL;
- virNodeDevicePtr device;
+ g_autoptr(virshNodeDevice) device = NULL;
bool ret = true;
virshControl *priv = ctl->privData;
@@ -715,7 +708,6 @@ cmdNodeDeviceReAttach(vshControl *ctl, const vshCmd *cmd)
ret = false;
}
- virNodeDeviceFree(device);
return ret;
}
@@ -747,7 +739,7 @@ static bool
cmdNodeDeviceReset(vshControl *ctl, const vshCmd *cmd)
{
const char *name = NULL;
- virNodeDevicePtr device;
+ g_autoptr(virshNodeDevice) device = NULL;
bool ret = true;
virshControl *priv = ctl->privData;
@@ -766,7 +758,6 @@ cmdNodeDeviceReset(vshControl *ctl, const vshCmd *cmd)
ret = false;
}
- virNodeDeviceFree(device);
return ret;
}
@@ -910,7 +901,7 @@ static const vshCmdOptDef opts_node_device_event[] = {
static bool
cmdNodeDeviceEvent(vshControl *ctl, const vshCmd *cmd)
{
- virNodeDevicePtr dev = NULL;
+ g_autoptr(virshNodeDevice) dev = NULL;
bool ret = false;
int eventId = -1;
int timeout = 0;
@@ -988,8 +979,6 @@ cmdNodeDeviceEvent(vshControl *ctl, const vshCmd *cmd)
if (eventId >= 0 &&
virConnectNodeDeviceEventDeregisterAny(priv->conn, eventId) < 0)
ret = false;
- if (dev)
- virNodeDeviceFree(dev);
return ret;
}
@@ -1020,7 +1009,7 @@ static const vshCmdOptDef opts_node_device_undefine[] = {
static bool
cmdNodeDeviceUndefine(vshControl *ctl, const vshCmd *cmd G_GNUC_UNUSED)
{
- virNodeDevice *dev = NULL;
+ g_autoptr(virshNodeDevice) dev = NULL;
bool ret = false;
const char *device_value = NULL;
@@ -1041,8 +1030,6 @@ cmdNodeDeviceUndefine(vshControl *ctl, const vshCmd *cmd
G_GNUC_UNUSED)
ret = true;
cleanup:
- if (dev)
- virNodeDeviceFree(dev);
return ret;
}
@@ -1071,7 +1058,7 @@ static const vshCmdOptDef opts_node_device_define[] = {
static bool
cmdNodeDeviceDefine(vshControl *ctl, const vshCmd *cmd G_GNUC_UNUSED)
{
- virNodeDevice *dev = NULL;
+ g_autoptr(virshNodeDevice) dev = NULL;
const char *from = NULL;
g_autofree char *buffer = NULL;
virshControl *priv = ctl->privData;
@@ -1089,7 +1076,6 @@ cmdNodeDeviceDefine(vshControl *ctl, const vshCmd *cmd
G_GNUC_UNUSED)
vshPrintExtra(ctl, _("Node device '%s' defined from
'%s'\n"),
virNodeDeviceGetName(dev), from);
- virNodeDeviceFree(dev);
return true;
}
@@ -1121,7 +1107,7 @@ static bool
cmdNodeDeviceStart(vshControl *ctl, const vshCmd *cmd)
{
const char *name = NULL;
- virNodeDevice *device;
+ g_autoptr(virshNodeDevice) device = NULL;
bool ret = true;
virshControl *priv = ctl->privData;
@@ -1140,7 +1126,6 @@ cmdNodeDeviceStart(vshControl *ctl, const vshCmd *cmd)
ret = false;
}
- virNodeDeviceFree(device);
return ret;
}
@@ -1175,7 +1160,7 @@ static const vshCmdOptDef opts_node_device_autostart[] = {
static bool
cmdNodeDeviceAutostart(vshControl *ctl, const vshCmd *cmd)
{
- virNodeDevice *dev = NULL;
+ g_autoptr(virshNodeDevice) dev = NULL;
bool ret = false;
const char *name = NULL;
int autostart;
@@ -1204,8 +1189,6 @@ cmdNodeDeviceAutostart(vshControl *ctl, const vshCmd *cmd)
ret = true;
cleanup:
- if (dev)
- virNodeDeviceFree(dev);
return ret;
}
@@ -1237,7 +1220,7 @@ static const vshCmdOptDef opts_node_device_info[] = {
static bool
cmdNodeDeviceInfo(vshControl *ctl, const vshCmd *cmd)
{
- virNodeDevicePtr device = NULL;
+ g_autoptr(virshNodeDevice) device = NULL;
const char *device_value = NULL;
bool ret = false;
int autostart;
@@ -1265,8 +1248,6 @@ cmdNodeDeviceInfo(vshControl *ctl, const vshCmd *cmd)
ret = true;
cleanup:
- if (device)
- virNodeDeviceFree(device);
return ret;
}
diff --git a/tools/virsh-util.c b/tools/virsh-util.c
index f7b649983e..5034f4773f 100644
--- a/tools/virsh-util.c
+++ b/tools/virsh-util.c
@@ -307,6 +307,17 @@ virshNetworkFree(virNetworkPtr network)
}
+void
+virshNodeDeviceFree(virNodeDevicePtr device)
+{
+ if (!device)
+ return;
+
+ vshSaveLibvirtHelperError();
+ virNodeDeviceFree(device); /* sc_prohibit_obj_free_apis_in_virsh */
+}
+
+
void
virshSecretFree(virSecretPtr secret)
{
diff --git a/tools/virsh-util.h b/tools/virsh-util.h
index e8df0a6618..06e311b21a 100644
--- a/tools/virsh-util.h
+++ b/tools/virsh-util.h
@@ -64,6 +64,11 @@ void
virshNetworkFree(virNetworkPtr network);
G_DEFINE_AUTOPTR_CLEANUP_FUNC(virshNetwork, virshNetworkFree);
+typedef virNodeDevice virshNodeDevice;
+void
+virshNodeDeviceFree(virNodeDevicePtr device);
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(virshNodeDevice, virshNodeDeviceFree);
+
typedef virSecret virshSecret;
void
virshSecretFree(virSecretPtr secret);
--
2.32.0
12:07 a.m.
New subject: [PATCH 7/8] virsh: Add wrapper for virNWFilterFree
Similarly to virshDomainFree add a wrapper for the snapshot object
freeing function.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
build-aux/syntax-check.mk | 2 +-
tools/virsh-completer-nwfilter.c | 3 ++-
tools/virsh-nwfilter.c | 22 +++++++---------------
tools/virsh-util.c | 11 +++++++++++
tools/virsh-util.h | 5 +++++
5 files changed, 26 insertions(+), 17 deletions(-)
diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk
index 6c230826bd..5daf5afcd0 100644
--- a/build-aux/syntax-check.mk
+++ b/build-aux/syntax-check.mk
@@ -868,7 +868,7 @@ sc_gettext_init:
$(_sc_search_regexp)
sc_prohibit_obj_free_apis_in_virsh:
- @prohibit='\bvir(Domain|DomainSnapshot|Interface|Network|NodeDevice|Secret|StoragePool|StorageVol)Free\b'
\
+ @prohibit='\bvir(Domain|DomainSnapshot|Interface|Network|NodeDevice|NWFilter|Secret|StoragePool|StorageVol)Free\b'
\
in_vc_files='virsh.*\.[ch]$$' \
exclude='sc_prohibit_obj_free_apis_in_virsh' \
halt='avoid using public virXXXFree in virsh, use virsh-prefixed wrappers
instead' \
diff --git a/tools/virsh-completer-nwfilter.c b/tools/virsh-completer-nwfilter.c
index 859f72f6e9..9850dbba5d 100644
--- a/tools/virsh-completer-nwfilter.c
+++ b/tools/virsh-completer-nwfilter.c
@@ -21,6 +21,7 @@
#include <config.h>
#include "virsh-completer-nwfilter.h"
+#include "virsh-util.h"
#include "viralloc.h"
#include "virsh.h"
#include "virstring.h"
@@ -56,7 +57,7 @@ virshNWFilterNameCompleter(vshControl *ctl,
ret = g_steal_pointer(&tmp);
for (i = 0; i < nnwfilters; i++)
- virNWFilterFree(nwfilters[i]);
+ virshNWFilterFree(nwfilters[i]);
g_free(nwfilters);
return ret;
}
diff --git a/tools/virsh-nwfilter.c b/tools/virsh-nwfilter.c
index 33164f623f..09ceaf6ec9 100644
--- a/tools/virsh-nwfilter.c
+++ b/tools/virsh-nwfilter.c
@@ -20,6 +20,7 @@
#include <config.h>
#include "virsh-nwfilter.h"
+#include "virsh-util.h"
#include "internal.h"
#include "viralloc.h"
@@ -91,7 +92,7 @@ static const vshCmdOptDef opts_nwfilter_define[] = {
static bool
cmdNWFilterDefine(vshControl *ctl, const vshCmd *cmd)
{
- virNWFilterPtr nwfilter;
+ g_autoptr(virshNWFilter) nwfilter = NULL;
const char *from = NULL;
bool ret = true;
g_autofree char *buffer = NULL;
@@ -115,7 +116,6 @@ cmdNWFilterDefine(vshControl *ctl, const vshCmd *cmd)
if (nwfilter != NULL) {
vshPrintExtra(ctl, _("Network filter %s defined from %s\n"),
virNWFilterGetName(nwfilter), from);
- virNWFilterFree(nwfilter);
} else {
vshError(ctl, _("Failed to define network filter from %s"), from);
ret = false;
@@ -149,7 +149,7 @@ static const vshCmdOptDef opts_nwfilter_undefine[] = {
static bool
cmdNWFilterUndefine(vshControl *ctl, const vshCmd *cmd)
{
- virNWFilterPtr nwfilter;
+ g_autoptr(virshNWFilter) nwfilter = NULL;
bool ret = true;
const char *name;
@@ -163,7 +163,6 @@ cmdNWFilterUndefine(vshControl *ctl, const vshCmd *cmd)
ret = false;
}
- virNWFilterFree(nwfilter);
return ret;
}
@@ -193,7 +192,7 @@ static const vshCmdOptDef opts_nwfilter_dumpxml[] = {
static bool
cmdNWFilterDumpXML(vshControl *ctl, const vshCmd *cmd)
{
- virNWFilterPtr nwfilter;
+ g_autoptr(virshNWFilter) nwfilter = NULL;
bool ret = true;
g_autofree char *dump = NULL;
@@ -207,7 +206,6 @@ cmdNWFilterDumpXML(vshControl *ctl, const vshCmd *cmd)
ret = false;
}
- virNWFilterFree(nwfilter);
return ret;
}
@@ -239,8 +237,7 @@ virshNWFilterListFree(struct virshNWFilterList *list)
if (list && list->filters) {
for (i = 0; i < list->nfilters; i++) {
- if (list->filters[i])
- virNWFilterFree(list->filters[i]);
+ virshNWFilterFree(list->filters[i]);
}
g_free(list->filters);
}
@@ -418,8 +415,8 @@ static bool
cmdNWFilterEdit(vshControl *ctl, const vshCmd *cmd)
{
bool ret = false;
- virNWFilterPtr nwfilter = NULL;
- virNWFilterPtr nwfilter_edited = NULL;
+ g_autoptr(virshNWFilter) nwfilter = NULL;
+ g_autoptr(virshNWFilter) nwfilter_edited = NULL;
virshControl *priv = ctl->privData;
nwfilter = virshCommandOptNWFilter(ctl, cmd, NULL);
@@ -445,11 +442,6 @@ cmdNWFilterEdit(vshControl *ctl, const vshCmd *cmd)
ret = true;
cleanup:
- if (nwfilter)
- virNWFilterFree(nwfilter);
- if (nwfilter_edited)
- virNWFilterFree(nwfilter_edited);
-
return ret;
}
diff --git a/tools/virsh-util.c b/tools/virsh-util.c
index 5034f4773f..fc2839d8fc 100644
--- a/tools/virsh-util.c
+++ b/tools/virsh-util.c
@@ -318,6 +318,17 @@ virshNodeDeviceFree(virNodeDevicePtr device)
}
+void
+virshNWFilterFree(virNWFilterPtr nwfilter)
+{
+ if (!nwfilter)
+ return;
+
+ vshSaveLibvirtHelperError();
+ virNWFilterFree(nwfilter); /* sc_prohibit_obj_free_apis_in_virsh */
+}
+
+
void
virshSecretFree(virSecretPtr secret)
{
diff --git a/tools/virsh-util.h b/tools/virsh-util.h
index 06e311b21a..065055ddb1 100644
--- a/tools/virsh-util.h
+++ b/tools/virsh-util.h
@@ -69,6 +69,11 @@ void
virshNodeDeviceFree(virNodeDevicePtr device);
G_DEFINE_AUTOPTR_CLEANUP_FUNC(virshNodeDevice, virshNodeDeviceFree);
+typedef virNWFilter virshNWFilter;
+void
+virshNWFilterFree(virNWFilterPtr nwfilter);
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(virshNWFilter, virshNWFilterFree);
+
typedef virSecret virshSecret;
void
virshSecretFree(virSecretPtr secret);
--
2.32.0
12:07 a.m.
New subject: [PATCH 8/8] virsh: Add wrapper for virStreamFree
Similarly to virshDomainFree add a wrapper for the snapshot object
freeing function.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
build-aux/syntax-check.mk | 2 +-
tools/virsh-console.c | 8 ++++----
tools/virsh-domain.c | 4 +---
tools/virsh-util.c | 12 ++++++++++++
tools/virsh-util.h | 5 +++++
tools/virsh-volume.c | 8 ++------
6 files changed, 25 insertions(+), 14 deletions(-)
diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk
index 5daf5afcd0..cb12b64532 100644
--- a/build-aux/syntax-check.mk
+++ b/build-aux/syntax-check.mk
@@ -868,7 +868,7 @@ sc_gettext_init:
$(_sc_search_regexp)
sc_prohibit_obj_free_apis_in_virsh:
- @prohibit='\bvir(Domain|DomainSnapshot|Interface|Network|NodeDevice|NWFilter|Secret|StoragePool|StorageVol)Free\b'
\
+ @prohibit='\bvir(Domain|DomainSnapshot|Interface|Network|NodeDevice|NWFilter|Secret|StoragePool|StorageVol|Stream)Free\b'
\
in_vc_files='virsh.*\.[ch]$$' \
exclude='sc_prohibit_obj_free_apis_in_virsh' \
halt='avoid using public virXXXFree in virsh, use virsh-prefixed wrappers
instead' \
diff --git a/tools/virsh-console.c b/tools/virsh-console.c
index 449619c95f..67ee608706 100644
--- a/tools/virsh-console.c
+++ b/tools/virsh-console.c
@@ -33,6 +33,7 @@
# include "internal.h"
# include "virsh.h"
# include "virsh-console.h"
+# include "virsh-util.h"
# include "virlog.h"
# include "virfile.h"
# include "viralloc.h"
@@ -117,8 +118,8 @@ virConsoleShutdown(virConsole *con,
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("cannot terminate console stream"));
}
- virStreamFree(con->st);
- con->st = NULL;
+
+ g_clear_pointer(&con->st, virshStreamFree);
}
VIR_FREE(con->streamToTerminal.data);
VIR_FREE(con->terminalToStream.data);
@@ -140,8 +141,7 @@ virConsoleDispose(void *obj)
{
virConsole *con = obj;
- if (con->st)
- virStreamFree(con->st);
+ virshStreamFree(con->st);
virCondDestroy(&con->cond);
virResetError(&con->error);
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index b1943b3875..0d4f7cc407 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -5540,7 +5540,7 @@ cmdScreenshot(vshControl *ctl, const vshCmd *cmd)
const char *name = NULL;
char *file = NULL;
int fd = -1;
- virStreamPtr st = NULL;
+ g_autoptr(virshStream) st = NULL;
unsigned int screen = 0;
unsigned int flags = 0; /* currently unused */
bool ret = false;
@@ -5610,8 +5610,6 @@ cmdScreenshot(vshControl *ctl, const vshCmd *cmd)
unlink(file);
if (generated)
VIR_FREE(file);
- if (st)
- virStreamFree(st);
VIR_FORCE_CLOSE(fd);
VIR_FREE(mime);
return ret;
diff --git a/tools/virsh-util.c b/tools/virsh-util.c
index fc2839d8fc..8fb617fa3c 100644
--- a/tools/virsh-util.c
+++ b/tools/virsh-util.c
@@ -362,6 +362,18 @@ virshStorageVolFree(virStorageVolPtr vol)
}
+
+void
+virshStreamFree(virStreamPtr stream)
+{
+ if (!stream)
+ return;
+
+ vshSaveLibvirtHelperError();
+ virStreamFree(stream); /* sc_prohibit_obj_free_apis_in_virsh */
+}
+
+
int
virshDomainGetXMLFromDom(vshControl *ctl,
virDomainPtr dom,
diff --git a/tools/virsh-util.h b/tools/virsh-util.h
index 065055ddb1..838935d5e8 100644
--- a/tools/virsh-util.h
+++ b/tools/virsh-util.h
@@ -89,6 +89,11 @@ void
virshStorageVolFree(virStorageVolPtr vol);
G_DEFINE_AUTOPTR_CLEANUP_FUNC(virshStorageVol, virshStorageVolFree);
+typedef virStream virshStream;
+void
+virshStreamFree(virStreamPtr stream);
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(virshStream, virshStreamFree);
+
int
virshDomainState(vshControl *ctl,
virDomainPtr dom,
diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c
index b896ebbbf9..70b6eac687 100644
--- a/tools/virsh-volume.c
+++ b/tools/virsh-volume.c
@@ -656,7 +656,7 @@ cmdVolUpload(vshControl *ctl, const vshCmd *cmd)
g_autoptr(virshStorageVol) vol = NULL;
bool ret = false;
int fd = -1;
- virStreamPtr st = NULL;
+ g_autoptr(virshStream) st = NULL;
const char *name = NULL;
unsigned long long offset = 0, length = 0;
virshControl *priv = ctl->privData;
@@ -731,8 +731,6 @@ cmdVolUpload(vshControl *ctl, const vshCmd *cmd)
ret = true;
cleanup:
- if (st)
- virStreamFree(st);
VIR_FORCE_CLOSE(fd);
return ret;
}
@@ -776,7 +774,7 @@ cmdVolDownload(vshControl *ctl, const vshCmd *cmd)
g_autoptr(virshStorageVol) vol = NULL;
bool ret = false;
int fd = -1;
- virStreamPtr st = NULL;
+ g_autoptr(virshStream) st = NULL;
const char *name = NULL;
unsigned long long offset = 0, length = 0;
bool created = false;
@@ -851,8 +849,6 @@ cmdVolDownload(vshControl *ctl, const vshCmd *cmd)
VIR_FORCE_CLOSE(fd);
if (!ret && created)
unlink(file);
- if (st)
- virStreamFree(st);
return ret;
}
--
2.32.0
4:35 p.m.
For the series:
Acked-by: Jonathon Jongsma <jjongsma(a)redhat.com>
On Mon, Sep 27, 2021 at 12:11 AM Michal Privoznik <mprivozn(a)redhat.com> wrote:
In this patchset I'm switching from virXXXFree to g_autoptr(). There are
still some left, but very rare occurrence:
libvirt.git $ git grep -o "vir[A-Z].*Free" -- tools/ | \
cut -d':' -f 2 | sort | uniq -c | sort -n
And of course, after these some functions could use subsequent cleanup,
e.g. because cleanup label collapsed to just return statement. But I
leave those for future work.
Michal Prívozník (8):
virsh-util.h: Fix ordering of virshXXXFree functions
virsh: Add wrapper for virInterfaceFree
virsh: Add wrapper for virStoragePoolFree
virsh: Add wrapper for virStorageVolFree
virsh: Add wrapper for virNetworkFree
virsh: Add wrapper for virNodeDeviceFree
virsh: Add wrapper for virNWFilterFree
virsh: Add wrapper for virStreamFree
build-aux/syntax-check.mk | 4 +-
tools/virsh-completer-interface.c | 3 +-
tools/virsh-completer-network.c | 8 +--
tools/virsh-completer-nodedev.c | 3 +-
tools/virsh-completer-nwfilter.c | 3 +-
tools/virsh-completer-pool.c | 3 +-
tools/virsh-completer-volume.c | 8 +--
tools/virsh-console.c | 8 +--
tools/virsh-domain.c | 10 +--
tools/virsh-interface.c | 55 +++++----------
tools/virsh-network.c | 75 ++++++--------------
tools/virsh-nodedev.c | 49 ++++----------
tools/virsh-nwfilter.c | 22 ++----
tools/virsh-pool.c | 67 ++++++------------
tools/virsh-util.c | 78 +++++++++++++++++++++
tools/virsh-util.h | 46 +++++++++++--
tools/virsh-volume.c | 109 ++++++++----------------------
17 files changed, 254 insertions(+), 297 deletions(-)
--
2.32.0
1179
days inactive
1187
days old
9 comments
2 participants
participants (2)
-
Jonathon Jongsma -
Michal Privoznik