Re: [PATCH 1/6] qapi: change 'unsigned special_features' to 'uint64_t features'

[Markus Armbruster <armbru@redhat.com>]

Daniel P. Berrangé <berrange@redhat.com> writes:

> The "special_features" field / parameter holds the subset of schema
> features that are for internal code use. Specifically 'DEPRECATED'
> and 'UNSTABLE'.
>
> This special casing of internal features is going to be removed, so
> prepare for that by renaming to 'features'. Using a fixed size type
> is also best practice for bit fields.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

[...]

>  static void forward_field_complete(Visitor *v, void *opaque)
> diff --git a/qapi/qapi-util.c b/qapi/qapi-util.c
> index 65a7d18437..3d849fe034 100644
> --- a/qapi/qapi-util.c
> +++ b/qapi/qapi-util.c
> @@ -37,19 +37,19 @@ static bool compat_policy_input_ok1(const char *adjective,
>      }
>  }
>  
> -bool compat_policy_input_ok(unsigned special_features,
> +bool compat_policy_input_ok(uint64_t features,
>                              const CompatPolicy *policy,
>                              ErrorClass error_class,
>                              const char *kind, const char *name,
>                              Error **errp)
>  {
> -    if ((special_features & 1u << QAPI_DEPRECATED)
> +    if ((features & 1u << QAPI_DEPRECATED)

Before the patch, both operands of & are unsigned.  The patch changes
one of them to uint64_t.  Shouldn't we change the other one, too?

>          && !compat_policy_input_ok1("Deprecated",
>                                      policy->deprecated_input,
>                                      error_class, kind, name, errp)) {
>          return false;
>      }
> -    if ((special_features & (1u << QAPI_UNSTABLE))
> +    if ((features & (1u << QAPI_UNSTABLE))

Likewise.

>          && !compat_policy_input_ok1("Unstable",
>                                      policy->unstable_input,
>                                      error_class, kind, name, errp)) {

[...]

> diff --git a/qapi/qobject-output-visitor.c b/qapi/qobject-output-visitor.c
> index 74770edd73..8902287caa 100644
> --- a/qapi/qobject-output-visitor.c
> +++ b/qapi/qobject-output-visitor.c
> @@ -210,13 +210,13 @@ static bool qobject_output_type_null(Visitor *v, const char *name,
>  }
>  
>  static bool qobject_output_policy_skip(Visitor *v, const char *name,
> -                                       unsigned special_features)
> +                                       uint64_t features)
>  {
>      CompatPolicy *pol = &v->compat_policy;
>  
> -    return ((special_features & 1u << QAPI_DEPRECATED)
> +    return ((features & 1u << QAPI_DEPRECATED)

Likewise.

>              && pol->deprecated_output == COMPAT_POLICY_OUTPUT_HIDE)
> -        || ((special_features & 1u << QAPI_UNSTABLE)
> +        || ((features & 1u << QAPI_UNSTABLE)

Likewise.

>              && pol->unstable_output == COMPAT_POLICY_OUTPUT_HIDE);
>  }
>  

[...]