Re: [PATCH 8/9] qapi: Factor out compat_policy_input_ok()

[Philippe Mathieu-Daudé]

On 10/25/21 07:25, Markus Armbruster wrote:
> The code to check policy for handling deprecated input is triplicated.
> Factor it out into compat_policy_input_ok() before I mess with it in
> the next commit.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  include/qapi/compat-policy.h |  7 +++++
>  qapi/qapi-visit-core.c       | 18 +++++--------
>  qapi/qmp-dispatch.c          | 51 +++++++++++++++++++++++++++---------
>  qapi/qobject-input-visitor.c | 19 +++-----------
>  4 files changed, 55 insertions(+), 40 deletions(-)

> diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
> index 8cca18c891..e29ade134c 100644
> --- a/qapi/qmp-dispatch.c
> +++ b/qapi/qmp-dispatch.c
> @@ -28,6 +28,40 @@
>  
>  CompatPolicy compat_policy;
>  
> +static bool compat_policy_input_ok1(const char *adjective,
> +                                    CompatPolicyInput policy,
> +                                    ErrorClass error_class,
> +                                    const char *kind, const char *name,
> +                                    Error **errp)
> +{
> +    switch (policy) {
> +    case COMPAT_POLICY_INPUT_ACCEPT:
> +        return true;
> +    case COMPAT_POLICY_INPUT_REJECT:
> +        error_set(errp, error_class, "%s %s %s disabled by policy",
> +                  adjective, kind, name);
> +        return false;
> +    case COMPAT_POLICY_INPUT_CRASH:
> +    default:
> +        abort();

g_assert_not_reached() provides a nicer user experience.

> +    }
> +}
> +
> +bool compat_policy_input_ok(unsigned special_features,
> +                            const CompatPolicy *policy,
> +                            ErrorClass error_class,
> +                            const char *kind, const char *name,
> +                            Error **errp)
> +{
> +    if ((special_features & 1u << QAPI_DEPRECATED)

Matter of taste, I find code using extract() easier to review:

  extract64(special_features, QAPI_DEPRECATED, 1)

> +        && !compat_policy_input_ok1("Deprecated",
> +                                    policy->deprecated_input,
> +                                    error_class, kind, name, errp)) {
> +        return false;
> +    }
> +    return true;
> +}

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>