[PATCH] tools: virsh-snapshot: refactor small functions

[Michal Prívozník]

On 9/17/21 3:23 PM, Kristina Hanicova wrote:
> This patch includes:
> * removal of dead code
> * simplifying nested if conditions
> * removal of unnecessary variables
> * usage of "direct" boolean return
> 
> Signed-off-by: Kristina Hanicova <khanicov at redhat.com>
> ---
>  tools/virsh-snapshot.c | 43 +++++++++++++++---------------------------
>  1 file changed, 15 insertions(+), 28 deletions(-)
> 
> diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
> index 2659b4340d..3bdad03df0 100644
> --- a/tools/virsh-snapshot.c
> +++ b/tools/virsh-snapshot.c
> @@ -771,7 +771,6 @@ virshSnapshotFilter(vshControl *ctl, virDomainSnapshotPtr snapshot,
>      g_autofree char *xml = NULL;
>      g_autoptr(xmlDoc) xmldoc = NULL;
>      g_autoptr(xmlXPathContext) ctxt = NULL;
> -    int ret = -1;
>      g_autofree char *state = NULL;
>  
>      if (!snapshot)
> @@ -796,20 +795,17 @@ virshSnapshotFilter(vshControl *ctl, virDomainSnapshotPtr snapshot,
>          return -1;
>      }
>      if (STREQ(state, "disk-snapshot")) {
> -        ret = ((flags & (VIR_DOMAIN_SNAPSHOT_LIST_DISK_ONLY |
> -                         VIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL)) ==
> -               (VIR_DOMAIN_SNAPSHOT_LIST_DISK_ONLY |
> -                VIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL));

So the only way this can be true is if both flags are set at the same
time. Since you're touching this, how about:

        return !!((flags & VIR_DOMAIN_SNAPSHOT_LIST_DISK_ONLY) &&
                  (flags & VIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL));

I find it more readable. I'll change it before pushing.

Reviewed-by: Michal Privoznik <mprivozn at redhat.com>

Michal