[PATCH 0/8] virsh: Use g_autoptr() for public types
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
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@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
Similarly to virshDomainFree add a wrapper for the snapshot object freeing function. Signed-off-by: Michal Privoznik <mprivozn@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
Similarly to virshDomainFree add a wrapper for the snapshot object freeing function. Signed-off-by: Michal Privoznik <mprivozn@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
Similarly to virshDomainFree add a wrapper for the snapshot object freeing function. Signed-off-by: Michal Privoznik <mprivozn@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
Similarly to virshDomainFree add a wrapper for the snapshot object freeing function. Signed-off-by: Michal Privoznik <mprivozn@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
Similarly to virshDomainFree add a wrapper for the snapshot object freeing function. Signed-off-by: Michal Privoznik <mprivozn@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
Similarly to virshDomainFree add a wrapper for the snapshot object freeing function. Signed-off-by: Michal Privoznik <mprivozn@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
Similarly to virshDomainFree add a wrapper for the snapshot object freeing function. Signed-off-by: Michal Privoznik <mprivozn@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
For the series: Acked-by: Jonathon Jongsma <jjongsma@redhat.com> On Mon, Sep 27, 2021 at 12:11 AM Michal Privoznik <mprivozn@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
participants (2)
-
Jonathon Jongsma -
Michal Privoznik