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@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxx>
Michal
