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

Eric Blake <eblake(a)redhat.com&gt; writes: > On Thu, Oct 28, 2021 at 12:25:16PM +0200, 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(a)redhat.com&gt; >> --- > >> +++ b/qapi/qapi-forward-visitor.c >> @@ -246,25 +246,27 @@ static void forward_field_optional(Visitor *v, const char *name, bool *present) >> visit_optional(ffv->target, name, present); >> } >> >> -static bool forward_field_deprecated_accept(Visitor *v, const char *name, >> - Error **errp) >> +static bool forward_field_policy_reject(Visitor *v, const char *name, >> + unsigned special_features, >> + Error **errp) >> { >> ForwardFieldVisitor *ffv = to_ffv(v); >> >> if (!forward_field_translate_name(ffv, &name, errp)) { >> return false; > > Should this return value be flipped? > >> } >> - return visit_deprecated_accept(ffv->target, name, errp); >> + return visit_policy_reject(ffv->target, name, special_features, errp); >> } >> >> -static bool forward_field_deprecated(Visitor *v, const char *name) >> +static bool forward_field_policy_skip(Visitor *v, const char *name, >> + unsigned special_features) >> { >> ForwardFieldVisitor *ffv = to_ffv(v); >> >> if (!forward_field_translate_name(ffv, &name, NULL)) { >> return false; > > and here too? Good catch!

These functions are called indirectly like if (visit_policy_reject(v, "values", 1u << QAPI_DEPRECATED, errp)) { return false; } if (!visit_policy_skip(v, "values", 1u << QAPI_DEPRECATED)) { if (!visit_type_strList(v, "values", &obj->values, errp)) { return false; } } visit_policy_reject() must return true when it sets an error, or else we call visit_policy_skip() with @errp pointing to non-null. Same argument for visit_policy_skip(). >> } >> - return visit_deprecated(ffv->target, name); >> + return visit_policy_skip(ffv->target, name, special_features); >> } >> > > Otherwise, the rest of the logic changes for flipped sense look right.