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 <quint...@redhat.com> (maintainer:Migration) >> Cc: Leonardo Bras <leob...@redhat.com> (reviewer:Migration) >> Cc: Peter Xu <pet...@redhat.com> (reviewer:Migration) >> Cc: qemu-triv...@nongnu.org >> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2018404 >> Signed-off-by: Laszlo Ersek <ler...@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 <m...@tls.msk.ru> Thanks! Laszlo
