[PATCH 0/2] qemu: remove else after return / goto
Kristina Hanicova (2): qemu: remove unnecessary else branches after return / goto qemu: remove else branches after return in qemuMigrationSrcPerform() src/qemu/qemu_domainjob.c | 3 +- src/qemu/qemu_migration.c | 59 +++++++++++++++++++-------------------- src/qemu/qemu_process.c | 4 +-- src/qemu/qemu_snapshot.c | 13 ++++----- 4 files changed, 38 insertions(+), 41 deletions(-) -- 2.35.1
I think the code looks cleaner without else branches. Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- src/qemu/qemu_domainjob.c | 3 +-- src/qemu/qemu_migration.c | 13 +++++++------ src/qemu/qemu_process.c | 4 ++-- src/qemu/qemu_snapshot.c | 13 ++++++------- 4 files changed, 16 insertions(+), 17 deletions(-) diff --git a/src/qemu/qemu_domainjob.c b/src/qemu/qemu_domainjob.c index cf1e093e22..6bf3a0ab42 100644 --- a/src/qemu/qemu_domainjob.c +++ b/src/qemu/qemu_domainjob.c @@ -153,8 +153,7 @@ qemuDomainAsyncJobPhaseFromString(qemuDomainAsyncJob job, if (STREQ(phase, "none")) return 0; - else - return -1; + return -1; } diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index f109598fb4..e16d579928 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -4286,9 +4286,10 @@ qemuMigrationSrcRun(virQEMUDriver *driver, rc = qemuMigrationSrcWaitForCompletion(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT, dconn, waitFlags); - if (rc == -2) { + if (rc == -2) goto error; - } else if (rc == -1) { + + if (rc == -1) { /* QEMU reported failed migration, nothing to cancel anymore */ cancel = false; goto error; @@ -4326,9 +4327,10 @@ qemuMigrationSrcRun(virQEMUDriver *driver, rc = qemuMigrationSrcWaitForCompletion(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT, dconn, waitFlags); - if (rc == -2) { + if (rc == -2) goto error; - } else if (rc == -1) { + + if (rc == -1) { /* QEMU reported failed migration, nothing to cancel anymore */ cancel = false; goto error; @@ -6189,8 +6191,7 @@ qemuMigrationDstErrorInit(virQEMUDriver *driver) driver->migrationErrors = virHashAtomicNew(qemuMigrationDstErrorFree); if (driver->migrationErrors) return 0; - else - return -1; + return -1; } /** diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 1ed60917ea..7a36f85d7e 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8700,8 +8700,8 @@ qemuProcessRefreshBlockjobs(virQEMUDriver *driver, if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV)) return qemuBlockJobRefreshJobs(driver, vm); - else - return qemuProcessRefreshLegacyBlockjobs(driver, vm); + + return qemuProcessRefreshLegacyBlockjobs(driver, vm); } diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index 5333730df1..212d37d3b4 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -583,13 +583,12 @@ qemuSnapshotPrepareDiskExternal(virDomainObj *vm, _("missing existing file for disk %s: %s"), snapdisk->name, snapdisk->src->path); return -1; - } else { - if (snapdisk->src->type == VIR_STORAGE_TYPE_BLOCK) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, - _("block device snapshot target '%s' doesn't exist"), - snapdisk->src->path); - return -1; - } + } + if (snapdisk->src->type == VIR_STORAGE_TYPE_BLOCK) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("block device snapshot target '%s' doesn't exist"), + snapdisk->src->path); + return -1; } } else { /* at this point VIR_STORAGE_TYPE_DIR was already rejected */ -- 2.35.1
On a Thursday in 2022, Kristina Hanicova wrote:
I think the code looks cleaner without else branches.
Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- src/qemu/qemu_domainjob.c | 3 +-- src/qemu/qemu_migration.c | 13 +++++++------ src/qemu/qemu_process.c | 4 ++-- src/qemu/qemu_snapshot.c | 13 ++++++------- 4 files changed, 16 insertions(+), 17 deletions(-)
diff --git a/src/qemu/qemu_domainjob.c b/src/qemu/qemu_domainjob.c index cf1e093e22..6bf3a0ab42 100644 --- a/src/qemu/qemu_domainjob.c +++ b/src/qemu/qemu_domainjob.c @@ -153,8 +153,7 @@ qemuDomainAsyncJobPhaseFromString(qemuDomainAsyncJob job,
if (STREQ(phase, "none")) return 0; - else - return -1; + return -1;
I think it would look even better with a blank line above the ending return.
}
@@ -6189,8 +6191,7 @@ qemuMigrationDstErrorInit(virQEMUDriver *driver) driver->migrationErrors = virHashAtomicNew(qemuMigrationDstErrorFree); if (driver->migrationErrors) return 0; - else - return -1; + return -1;
Same here.
}
/** diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 1ed60917ea..7a36f85d7e 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8700,8 +8700,8 @@ qemuProcessRefreshBlockjobs(virQEMUDriver *driver,
if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV)) return qemuBlockJobRefreshJobs(driver, vm); - else - return qemuProcessRefreshLegacyBlockjobs(driver, vm); + + return qemuProcessRefreshLegacyBlockjobs(driver, vm); }
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index 5333730df1..212d37d3b4 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -583,13 +583,12 @@ qemuSnapshotPrepareDiskExternal(virDomainObj *vm, _("missing existing file for disk %s: %s"), snapdisk->name, snapdisk->src->path); return -1; - } else { - if (snapdisk->src->type == VIR_STORAGE_TYPE_BLOCK) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, - _("block device snapshot target '%s' doesn't exist"), - snapdisk->src->path); - return -1; - } + }
This also looks crowded without the empty line.
+ if (snapdisk->src->type == VIR_STORAGE_TYPE_BLOCK) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("block device snapshot target '%s' doesn't exist"), + snapdisk->src->path); + return -1; } } else { /* at this point VIR_STORAGE_TYPE_DIR was already rejected */
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- src/qemu/qemu_migration.c | 46 +++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 24 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index e16d579928..d8b00aca87 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -5482,31 +5482,29 @@ qemuMigrationSrcPerform(virQEMUDriver *driver, cookiein, cookieinlen, cookieout, cookieoutlen, flags, dname, resource, v3proto); - } else { - if (dconnuri) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("Unexpected dconnuri parameter with non-peer2peer migration")); - return -1; - } - - if (v3proto) { - return qemuMigrationSrcPerformPhase(driver, conn, vm, persist_xml, uri, - graphicsuri, - nmigrate_disks, migrate_disks, - migParams, - cookiein, cookieinlen, - cookieout, cookieoutlen, - flags, resource, nbdURI); - } else { - return qemuMigrationSrcPerformJob(driver, conn, vm, xmlin, persist_xml, NULL, - uri, graphicsuri, listenAddress, - nmigrate_disks, migrate_disks, nbdPort, - nbdURI, migParams, - cookiein, cookieinlen, - cookieout, cookieoutlen, flags, - dname, resource, v3proto); - } } + if (dconnuri) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Unexpected dconnuri parameter with non-peer2peer migration")); + return -1; + } + + if (v3proto) { + return qemuMigrationSrcPerformPhase(driver, conn, vm, persist_xml, uri, + graphicsuri, + nmigrate_disks, migrate_disks, + migParams, + cookiein, cookieinlen, + cookieout, cookieoutlen, + flags, resource, nbdURI); + } + return qemuMigrationSrcPerformJob(driver, conn, vm, xmlin, persist_xml, NULL, + uri, graphicsuri, listenAddress, + nmigrate_disks, migrate_disks, nbdPort, + nbdURI, migParams, + cookiein, cookieinlen, + cookieout, cookieoutlen, flags, + dname, resource, v3proto); } static int -- 2.35.1
On a Thursday in 2022, Kristina Hanicova wrote:
Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- src/qemu/qemu_migration.c | 46 +++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 24 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index e16d579928..d8b00aca87 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -5482,31 +5482,29 @@ qemuMigrationSrcPerform(virQEMUDriver *driver, cookiein, cookieinlen, cookieout, cookieoutlen, flags, dname, resource, v3proto); - } else { - if (dconnuri) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("Unexpected dconnuri parameter with non-peer2peer migration")); - return -1; - } - - if (v3proto) { - return qemuMigrationSrcPerformPhase(driver, conn, vm, persist_xml, uri, - graphicsuri, - nmigrate_disks, migrate_disks, - migParams, - cookiein, cookieinlen, - cookieout, cookieoutlen, - flags, resource, nbdURI); - } else { - return qemuMigrationSrcPerformJob(driver, conn, vm, xmlin, persist_xml, NULL, - uri, graphicsuri, listenAddress, - nmigrate_disks, migrate_disks, nbdPort, - nbdURI, migParams, - cookiein, cookieinlen, - cookieout, cookieoutlen, flags, - dname, resource, v3proto); - } }
I'm missing a blank line here.
+ if (dconnuri) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Unexpected dconnuri parameter with non-peer2peer migration")); + return -1; + } + + if (v3proto) { + return qemuMigrationSrcPerformPhase(driver, conn, vm, persist_xml, uri, + graphicsuri, + nmigrate_disks, migrate_disks, + migParams, + cookiein, cookieinlen, + cookieout, cookieoutlen, + flags, resource, nbdURI); + }
And here.
+ return qemuMigrationSrcPerformJob(driver, conn, vm, xmlin, persist_xml, NULL, + uri, graphicsuri, listenAddress, + nmigrate_disks, migrate_disks, nbdPort, + nbdURI, migParams, + cookiein, cookieinlen, + cookieout, cookieoutlen, flags, + dname, resource, v3proto); }
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Kristina Hanicova