In a few cases we call a public API, wrapped in an if() statement with both branches written out explicitly. The error branch jumps onto cleanup label, while the successful prints out a message. Right after these ifs there's 'ret = true;' and the cleanup label. The code is a bit more readable if only the error branch is kept and printing happens at the same level as setting the ret variable. Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx> --- tools/virsh-nodedev.c | 5 ++--- tools/virsh-volume.c | 32 +++++++++++++++----------------- 2 files changed, 17 insertions(+), 20 deletions(-) diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c index 1ad8db7a3f..424865962f 100644 --- a/tools/virsh-nodedev.c +++ b/tools/virsh-nodedev.c @@ -1024,13 +1024,12 @@ cmdNodeDeviceUndefine(vshControl *ctl, const vshCmd *cmd G_GNUC_UNUSED) if (!dev) goto cleanup; - if (virNodeDeviceUndefine(dev, 0) == 0) { - vshPrintExtra(ctl, _("Undefined node device '%s'\n"), device_value); - } else { + if (virNodeDeviceUndefine(dev, 0) < 0) { vshError(ctl, _("Failed to undefine node device '%s'"), device_value); goto cleanup; } + vshPrintExtra(ctl, _("Undefined node device '%s'\n"), device_value); ret = true; cleanup: return ret; diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c index 4c8e841701..12773d900b 100644 --- a/tools/virsh-volume.c +++ b/tools/virsh-volume.c @@ -411,14 +411,14 @@ cmdVolCreate(vshControl *ctl, const vshCmd *cmd) goto cleanup; } - if ((vol = virStorageVolCreateXML(pool, buffer, flags))) { - vshPrintExtra(ctl, _("Vol %s created from %s\n"), - virStorageVolGetName(vol), from); - ret = true; - } else { + if (!(vol = virStorageVolCreateXML(pool, buffer, flags))) { vshError(ctl, _("Failed to create vol from %s"), from); + goto cleanup; } + vshPrintExtra(ctl, _("Vol %s created from %s\n"), + virStorageVolGetName(vol), from); + ret = true; cleanup: return ret; } @@ -489,14 +489,13 @@ cmdVolCreateFrom(vshControl *ctl, const vshCmd *cmd) newvol = virStorageVolCreateXMLFrom(pool, buffer, inputvol, flags); - if (newvol != NULL) { - vshPrintExtra(ctl, _("Vol %s created from input vol %s\n"), - virStorageVolGetName(newvol), virStorageVolGetName(inputvol)); - } else { + if (!newvol) { vshError(ctl, _("Failed to create vol from %s"), from); goto cleanup; } + vshPrintExtra(ctl, _("Vol %s created from input vol %s\n"), + virStorageVolGetName(newvol), virStorageVolGetName(inputvol)); ret = true; cleanup: return ret; @@ -1147,20 +1146,19 @@ cmdVolResize(vshControl *ctl, const vshCmd *cmd) goto cleanup; } - if (virStorageVolResize(vol, capacity, flags) == 0) { - vshPrintExtra(ctl, - delta ? _("Size of volume '%s' successfully changed by %s\n") - : _("Size of volume '%s' successfully changed to %s\n"), - virStorageVolGetName(vol), capacityStr); - ret = true; - } else { + if (virStorageVolResize(vol, capacity, flags) < 0) { vshError(ctl, delta ? _("Failed to change size of volume '%s' by %s") : _("Failed to change size of volume '%s' to %s"), virStorageVolGetName(vol), capacityStr); - ret = false; + goto cleanup; } + vshPrintExtra(ctl, + delta ? _("Size of volume '%s' successfully changed by %s\n") + : _("Size of volume '%s' successfully changed to %s\n"), + virStorageVolGetName(vol), capacityStr); + ret = true; cleanup: return ret; } -- 2.32.0