Philippe Mathieu-Daudé
2021-Oct-25 12:13 UTC
[Libguestfs] [PATCH 8/9] qapi: Factor out compat_policy_input_ok()
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.> + } > +} > + > +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 at redhat.com>
Markus Armbruster
2021-Oct-26 09:46 UTC
[Libguestfs] [PATCH 8/9] qapi: Factor out compat_policy_input_ok()
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!