Re: [PATCH v2 5/9] qapi: Generalize struct member policy checking

[Philippe Mathieu-Daudé]

On 10/28/21 12:25, Markus Armbruster wrote:
> The generated visitor functions call visit_deprecated_accept() and
> visit_deprecated() when visiting a struct member with special feature
> flag 'deprecated'.  This makes the feature flag visible to the actual
> visitors.  I want to make feature flag 'unstable' visible there as
> well, so I can add policy for it.
> 
> To let me make it visible, replace these functions by
> visit_policy_reject() and visit_policy_skip(), which take the member's
> special features as an argument.  Note that the new functions have the
> opposite sense, i.e. the return value flips.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  include/qapi/visitor-impl.h   |  6 ++++--
>  include/qapi/visitor.h        | 17 +++++++++++++----
>  qapi/qapi-forward-visitor.c   | 16 +++++++++-------
>  qapi/qapi-visit-core.c        | 22 ++++++++++++----------
>  qapi/qobject-input-visitor.c  | 15 ++++++++++-----
>  qapi/qobject-output-visitor.c |  9 ++++++---
>  qapi/trace-events             |  4 ++--
>  scripts/qapi/visit.py         | 14 +++++++-------
>  8 files changed, 63 insertions(+), 40 deletions(-)

> -static bool qobject_input_deprecated_accept(Visitor *v, const char *name,
> -                                            Error **errp)
> +static bool qobject_input_policy_reject(Visitor *v, const char *name,
> +                                        unsigned special_features,
> +                                        Error **errp)
>  {
> +    if (!(special_features & 1u << QAPI_DEPRECATED)) {
> +        return false;
> +    }
> +
>      switch (v->compat_policy.deprecated_input) {
>      case COMPAT_POLICY_INPUT_ACCEPT:
> -        return true;
> +        return false;
>      case COMPAT_POLICY_INPUT_REJECT:
>          error_setg(errp, "Deprecated parameter '%s' disabled by policy",
>                     name);
> -        return false;
> +        return true;
>      case COMPAT_POLICY_INPUT_CRASH:

Clearer as:

           abort();
       default:
           g_assert_not_reached();

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

>      default:
>          abort();