It cuts back on those awkward, duplicated !(has_resume && resume)
expressions.
Cc: Juan Quintela <quintela@redhat.com> (maintainer:Migration)
Cc: Leonardo Bras <leobras@redhat.com> (reviewer:Migration)
Cc: Peter Xu <peterx@redhat.com> (reviewer:Migration)
Cc: qemu-trivial@nongnu.org
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2018404
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
migration/migration.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index 096e8191d15c..a60a5acee533 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1637,6 +1637,7 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
bool has_inc, bool inc, bool has_detach, bool detach,
bool has_resume, bool resume, Error **errp)
{
+ bool resume_requested;
Error *local_err = NULL;
MigrationState *s = migrate_get_current();
const char *p = NULL;
@@ -1646,13 +1647,14 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
return;
}
+ resume_requested = has_resume && resume;
if (!migrate_prepare(s, has_blk && blk, has_inc && inc,
- has_resume && resume, errp)) {
+ resume_requested, errp)) {
/* Error detected, put into errp */
return;
}
- if (!(has_resume && resume)) {
+ if (!resume_requested) {
if (!yank_register_instance(MIGRATION_YANK_INSTANCE, errp)) {
return;
}
@@ -1671,7 +1673,7 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
} else if (strstart(uri, "fd:", &p)) {
fd_start_outgoing_migration(s, p, &local_err);
} else {
- if (!(has_resume && resume)) {
+ if (!resume_requested) {
yank_unregister_instance(MIGRATION_YANK_INSTANCE);
}
error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "uri",
@@ -1683,7 +1685,7 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
}
if (local_err) {
- if (!(has_resume && resume)) {
+ if (!resume_requested) {
yank_unregister_instance(MIGRATION_YANK_INSTANCE);
}
migrate_fd_error(s, local_err);
On 6/7/23 12:29, Laszlo Ersek wrote: > It cuts back on those awkward, duplicated !(has_resume && resume) > expressions. > > Cc: Juan Quintela <quintela@redhat.com> (maintainer:Migration) > Cc: Leonardo Bras <leobras@redhat.com> (reviewer:Migration) > Cc: Peter Xu <peterx@redhat.com> (reviewer:Migration) > Cc: qemu-trivial@nongnu.org > Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2018404 > Signed-off-by: Laszlo Ersek <lersek@redhat.com> > --- > migration/migration.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
06.07.2023 13:29, Laszlo Ersek пишет:
> It cuts back on those awkward, duplicated !(has_resume && resume)
> expressions.
>
> Cc: Juan Quintela <quintela@redhat.com> (maintainer:Migration)
> Cc: Leonardo Bras <leobras@redhat.com> (reviewer:Migration)
> Cc: Peter Xu <peterx@redhat.com> (reviewer:Migration)
> Cc: qemu-trivial@nongnu.org
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2018404
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> migration/migration.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index 096e8191d15c..a60a5acee533 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -1637,6 +1637,7 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
> bool has_inc, bool inc, bool has_detach, bool detach,
> bool has_resume, bool resume, Error **errp)
> {
> + bool resume_requested;
> Error *local_err = NULL;
> MigrationState *s = migrate_get_current();
> const char *p = NULL;
> @@ -1646,13 +1647,14 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
> return;
> }
>
> + resume_requested = has_resume && resume;
Dunno if it's worth it or cleaner, but it can be reduced to
if (!has_resume) resume = false;
and checking for only resume below this point.
In other words, there's no need for an additional local var.
All other params (has_inc & inc, has_detach_detach etc) are like this
too.
Anyway,
Reviewed-by: Michael Tokarev <mjt@tls.msk.ru>
/mjt
On 7/6/23 15:28, Michael Tokarev wrote:
> 06.07.2023 13:29, Laszlo Ersek пишет:
>> It cuts back on those awkward, duplicated !(has_resume && resume)
>> expressions.
>>
>> Cc: Juan Quintela <quintela@redhat.com> (maintainer:Migration)
>> Cc: Leonardo Bras <leobras@redhat.com> (reviewer:Migration)
>> Cc: Peter Xu <peterx@redhat.com> (reviewer:Migration)
>> Cc: qemu-trivial@nongnu.org
>> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2018404
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>> ---
>> migration/migration.c | 10 ++++++----
>> 1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/migration/migration.c b/migration/migration.c
>> index 096e8191d15c..a60a5acee533 100644
>> --- a/migration/migration.c
>> +++ b/migration/migration.c
>> @@ -1637,6 +1637,7 @@ void qmp_migrate(const char *uri, bool has_blk,
>> bool blk,
>> bool has_inc, bool inc, bool has_detach, bool detach,
>> bool has_resume, bool resume, Error **errp)
>> {
>> + bool resume_requested;
>> Error *local_err = NULL;
>> MigrationState *s = migrate_get_current();
>> const char *p = NULL;
>> @@ -1646,13 +1647,14 @@ void qmp_migrate(const char *uri, bool
>> has_blk, bool blk,
>> return;
>> }
>> + resume_requested = has_resume && resume;
>
> Dunno if it's worth it or cleaner, but it can be reduced to
>
> if (!has_resume) resume = false;
>
> and checking for only resume below this point.
> In other words, there's no need for an additional local var.
I vehemently disagree with overwriting (input) parameters. One situation
where that practice is a disaster is single-stepping through the
function in an interactive debugger. You won't see the actual argument
the function was originally called with.
I know it's sometimes comfortable to just reuse a "count" input
paramater as a loop index that runs to zero -- I resist that too, it's a
trap (for the same reason), IMO.
>
> All other params (has_inc & inc, has_detach_detach etc) are like this
> too.
>
> Anyway,
>
> Reviewed-by: Michael Tokarev <mjt@tls.msk.ru>
Thanks!
Laszlo
Laszlo Ersek <lersek@redhat.com> wrote: > It cuts back on those awkward, duplicated !(has_resume && resume) > expressions. > > Cc: Juan Quintela <quintela@redhat.com> (maintainer:Migration) > Cc: Leonardo Bras <leobras@redhat.com> (reviewer:Migration) > Cc: Peter Xu <peterx@redhat.com> (reviewer:Migration) > Cc: qemu-trivial@nongnu.org > Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2018404 > Signed-off-by: Laszlo Ersek <lersek@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> Thanks. queued.
© 2016 - 2025 Red Hat, Inc.