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

[Markus Armbruster]

Philippe Mathieu-Daudé <philmd at redhat.com> writes:

> 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 at 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.

I find it hard to care for making the experience of a crash that should
never ever happen nicer :)

>> +    }
>> +}
>> +
>> +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)

I agree for width > 1.

>> +        && !compat_policy_input_ok1("Deprecated",
>> +                                    policy->deprecated_input,
>> +                                    error_class, kind, name, errp)) {
>> +        return false;
>> +    }
>> +    return true;
>> +}
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd at redhat.com>

Thanks!