Re: [PATCH v2 3/4] virsh: remove variable 'ret' and 'inactive'
On 9/24/21 1:49 AM, Kristina Hanicova wrote:
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
---
tools/virsh-interface.c | 15 ++++++---------
tools/virsh-network.c | 17 ++++++-----------
2 files changed, 12 insertions(+), 20 deletions(-)
diff --git a/tools/virsh-interface.c b/tools/virsh-interface.c
index 4bcc59b580..f402119b68 100644
--- a/tools/virsh-interface.c
+++ b/tools/virsh-interface.c
@@ -485,26 +485,23 @@ static bool
cmdInterfaceDumpXML(vshControl *ctl, const vshCmd *cmd)
{
virInterfacePtr iface;
1: this ^^^
- bool ret = true;
g_autofree char *dump = NULL;
unsigned int flags = 0;
- bool inactive = vshCommandOptBool(cmd, "inactive");
- if (inactive)
+ if (vshCommandOptBool(cmd, "inactive"))
flags |= VIR_INTERFACE_XML_INACTIVE;
if (!(iface = virshCommandOptInterface(ctl, cmd, NULL)))
return false;
- dump = virInterfaceGetXMLDesc(iface, flags);
- if (dump != NULL) {
- vshPrint(ctl, "%s", dump);
- } else {
- ret = false;
+ if (!(dump = virInterfaceGetXMLDesc(iface, flags))) {
+ virInterfaceFree(iface);
+ return false;
}
+ vshPrint(ctl, "%s", dump);
virInterfaceFree(iface);
- return ret;
+ return true;
I wonder whether we can declare an g_autoptr() cleanup for these public
APIs. We can't do that in public headers BUT we could do it in something
private, that's widely available (internal.h perhaps?). That way we
could turn [1] into g_autoptr() and drop both these free calls.
Obviously, that's orthogonal to what you are doing here and as such must
go into a standalone patchset. Just an idea I had.
Michal