Miguel Di Ciurcio Filho <miguel.fi...@gmail.com> writes: > The bdrv_snaphost_find() returns zero in case it finds an snapshot or -ENOENT > in > case it doesn't. > > Checking returning values as >= zero doesn't make sense.
Debatable. "RETVAL < 0" is an idiomatic check for error. "RETVAL >= 0" is merely its negation. Anyway, I do prefer code like this ret = do_something(); if (ret < 0) { handle error... } do more... over ret = do_something(); if (ret >= 0) { do more... } > > Signed-off-by: Miguel Di Ciurcio Filho <miguel.fi...@gmail.com> > --- > savevm.c | 7 ++++--- > 1 files changed, 4 insertions(+), 3 deletions(-) > > diff --git a/savevm.c b/savevm.c > index 7a1de3c..6c6adb0 100644 > --- a/savevm.c > +++ b/savevm.c > @@ -1768,7 +1768,7 @@ static int del_existing_snapshots(Monitor *mon, const > char *name) > bs = NULL; > while ((bs = bdrv_next(bs))) { > if (bdrv_can_snapshot(bs) && > - bdrv_snapshot_find(bs, snapshot, name) >= 0) > + bdrv_snapshot_find(bs, snapshot, name) == 0) > { > ret = bdrv_snapshot_delete(bs, name); > if (ret < 0) { > @@ -1948,8 +1948,9 @@ int load_vmstate(const char *name) > > /* Don't even try to load empty VM states */ > ret = bdrv_snapshot_find(bs, &sn, name); > - if ((ret >= 0) && (sn.vm_state_size == 0)) > - return -EINVAL; > + if ((ret == 0) && (sn.vm_state_size == 0)) { > + return -EINVAL; > + } > > /* restore the VM state */ > f = qemu_fopen_bdrv(bs, 0); I wonder what happens if bdrv_snapshot_find() fails. Why is it safe to ignore that and continue? do_savevm() has another one: ret = bdrv_snapshot_find(bs, old_sn, name); if (ret >= 0) { pstrcpy(sn->name, sizeof(sn->name), old_sn->name); pstrcpy(sn->id_str, sizeof(sn->id_str), old_sn->id_str); } else { pstrcpy(sn->name, sizeof(sn->name), name); }