Philippe Mathieu-Daudé <philmd(a)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(a)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(a)redhat.com>
Thanks!