[PATCH v2 3/4] virsh: remove variable 'ret' and 'inactive'

[Michal Prívozník]

On 9/24/21 1:49 AM, Kristina Hanicova wrote:
> Signed-off-by: Kristina Hanicova <khanicov at 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