This patch rewrites the pattern using early return where it is not needed. Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- src/qemu/qemu_alias.c | 41 ++++++++++++++++------------------------- 1 file changed, 16 insertions(+), 25 deletions(-) diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c index 5e35f43614..6f664fcd84 100644 --- a/src/qemu/qemu_alias.c +++ b/src/qemu/qemu_alias.c @@ -361,10 +361,9 @@ static int qemuAssignDeviceSoundAlias(virDomainSoundDef *sound, int idx) { - if (sound->info.alias) - return 0; + if (!sound->info.alias) + sound->info.alias = g_strdup_printf("sound%d", idx); - sound->info.alias = g_strdup_printf("sound%d", idx); return 0; } @@ -373,10 +372,9 @@ static int qemuAssignDeviceVideoAlias(virDomainVideoDef *video, int idx) { - if (video->info.alias) - return 0; + if (!video->info.alias) + video->info.alias = g_strdup_printf("video%d", idx); - video->info.alias = g_strdup_printf("video%d", idx); return 0; } @@ -385,10 +383,9 @@ static int qemuAssignDeviceHubAlias(virDomainHubDef *hub, int idx) { - if (hub->info.alias) - return 0; + if (!hub->info.alias) + hub->info.alias = g_strdup_printf("hub%d", idx); - hub->info.alias = g_strdup_printf("hub%d", idx); return 0; } @@ -397,10 +394,9 @@ static int qemuAssignDeviceSmartcardAlias(virDomainSmartcardDef *smartcard, int idx) { - if (smartcard->info.alias) - return 0; + if (!smartcard->info.alias) + smartcard->info.alias = g_strdup_printf("smartcard%d", idx); - smartcard->info.alias = g_strdup_printf("smartcard%d", idx); return 0; } @@ -409,10 +405,9 @@ static int qemuAssignDeviceMemballoonAlias(virDomainMemballoonDef *memballoon, int idx) { - if (memballoon->info.alias) - return 0; + if (!memballoon->info.alias) + memballoon->info.alias = g_strdup_printf("balloon%d", idx); - memballoon->info.alias = g_strdup_printf("balloon%d", idx); return 0; } @@ -421,10 +416,9 @@ static int qemuAssignDeviceTPMAlias(virDomainTPMDef *tpm, int idx) { - if (tpm->info.alias) - return 0; + if (!tpm->info.alias) + tpm->info.alias = g_strdup_printf("tpm%d", idx); - tpm->info.alias = g_strdup_printf("tpm%d", idx); return 0; } @@ -586,10 +580,8 @@ qemuAssignDeviceWatchdogAlias(virDomainWatchdogDef *watchdog) { /* Currently, there's just one watchdog per domain */ - if (watchdog->info.alias) - return 0; - - watchdog->info.alias = g_strdup("watchdog0"); + if (!watchdog->info.alias) + watchdog->info.alias = g_strdup("watchdog0"); return 0; } @@ -621,9 +613,8 @@ qemuAssignDeviceInputAlias(virDomainDef *def, int qemuAssignDeviceVsockAlias(virDomainVsockDef *vsock) { - if (vsock->info.alias) - return 0; - vsock->info.alias = g_strdup("vsock0"); + if (!vsock->info.alias) + vsock->info.alias = g_strdup("vsock0"); return 0; } -- 2.31.1

On a Tuesday in 2021, Kristina Hanicova wrote:

I have seen this pattern a lot in the project, so I decided to rewrite code I stumbled upon to the same pattern as well.

Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- src/qemu/qemu_driver.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1959b639da..b938687189 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16231,10 +16231,9 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorSetBlockIoThrottle(priv->mon, drivealias, qdevid, &info); - if (qemuDomainObjExitMonitor(driver, vm) < 0) - ret = -1; - if (ret < 0) + if (qemuDomainObjExitMonitor(driver, vm) < 0 || ret < 0) goto endjob;

This is not equivalent code. Before, if qemuDomainObjExitMonitor failed, we would set ret to -1 before goto endjob. Now, if qemuDomainObjExitMonitor fails, we will return whatever qemuMonitorSetBlockIoThrottle returned.

+ ret = -1;

This is an odd assignment. Introduced by 87ee705183241a42ffd36d9f5d3934cacf91c343 it resets 'ret' to its original state after we've used it for the return value of 'qemuMonitorSetBlockIoThrottle'. Introducing a separate 'rc' or 'rv' variable for that purpose would solve the oddness and remove the need to reset ret back to -1 if qemuDomainObjExitMonitor failed.

}

@@ -16362,9 +16361,7 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, } qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorGetBlockIoThrottle(priv->mon, drivealias, qdevid, &reply); - if (qemuDomainObjExitMonitor(driver, vm) < 0) - goto endjob; - if (ret < 0) + if (qemuDomainObjExitMonitor(driver, vm) < 0 || ret < 0)

Here, the existing code is buggy. It can return 0 from qemuMonitorGetBlockIoThrottle even if qemuDomainObjExitMonitor failed.

goto endjob; }

@@ -17375,10 +17372,7 @@ qemuDomainSetTime(virDomainPtr dom, if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_RTC_RESET_REINJECTION)) { qemuDomainObjEnterMonitor(driver, vm); rv = qemuMonitorRTCResetReinjection(priv->mon); - if (qemuDomainObjExitMonitor(driver, vm) < 0) - goto endjob; - - if (rv < 0) + if (qemuDomainObjExitMonitor(driver, vm) < 0 || rv < 0) goto endjob; }

This one looks good. Jano