Re: [PATCH v3 15/20] migration: Rewrite migrate_set_parameter_completion using QDict

[Markus Armbruster <armbru@redhat.com>]

Fabiano Rosas <farosas@suse.de> writes:

> We're moving towards having no open-coded access to the
> MigrationParameters members. Instances of parameters enumeration are
> being removed.
>
> The migrate_set_parameter_completion function is one of the last users
> of the MigrationParameter enum. Rewrite that code using an output
> visitor and QDict instead.
>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
>  migration/migration-hmp-cmds.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
> index 00d2d310af..c63d7f1ccb 100644
> --- a/migration/migration-hmp-cmds.c
> +++ b/migration/migration-hmp-cmds.c
> @@ -693,14 +693,20 @@ void migrate_set_capability_completion(ReadLineState *rs, int nb_args,
>  void migrate_set_parameter_completion(ReadLineState *rs, int nb_args,
>                                        const char *str)
>  {
> +    g_autoptr(QDict) d = NULL;
> +    const QDictEntry *e;
>      size_t len;
>  
> +    /* Temporarily borrow the global parameters */

What do you mean by "borrow"?

> +    d = migrate_params_to_dict(&migrate_get_current()->parameters,
> +                               &error_abort);

@d is unused unless @nb_args == 2.  Move the call into the conditional?

> +
>      len = strlen(str);
>      readline_set_completion_index(rs, len);
>      if (nb_args == 2) {
> -        int i;
> -        for (i = 0; i < MIGRATION_PARAMETER__MAX; i++) {
> -            readline_add_completion_of(rs, str, MigrationParameter_str(i));
> +        for (e = qdict_first(d); e; e = qdict_next(d, e)) {
> +            const char *key = qdict_entry_key(e);
> +            readline_add_completion_of(rs, str, key);
>          }
>      }
>  }

This relies on all parameters being present in the value of
migrate_get_current()->parameters.  Worth a comment?