[PATCH] tools: virsh-snapshot: refactor small functions
Peter Krempa
pkrempa at redhat.com
Mon Sep 20 08:14:26 UTC 2021
On Mon, Sep 20, 2021 at 09:36:25 +0200, Michal Prívozník wrote:
> 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));
Note that the double negation trick to force a cast to boolean isn't
necessary here, since the result of
(flags & VIR_DOMAIN_SNAPSHOT_LIST_DISK_ONLY) && (flags & VIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL)
is already a boolean (result of 'A && B')
More information about the libvir-list
mailing list