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

[Markus Armbruster]

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

> On 10/25/21 07: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 at 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(-)
>
>> diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
>> index 71b24a4429..fda485614b 100644
>> --- a/qapi/qobject-input-visitor.c
>> +++ b/qapi/qobject-input-visitor.c
>> @@ -662,16 +662,21 @@ static void qobject_input_optional(Visitor *v, const char *name, bool *present)
>>      *present = true;
>>  }
>>  
>> -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)) {
>
> Unreachable =) Proof than extract() is safer :P

Good eyes, thank you!

I actually like extract & desposit macros when the width is greater than
one.  Then, the longhand C code is illegible anyway, and having to
remember what the macros mean is no worse.

For width 1 it feels like a wash.  Universal use of the macros could
build familiarity and thus tip the balance.

I count more than a thousand instances of '& (1 <<'.

I wasn't even aware the macros existed in QEMU[*].

>
>> +        return false;
>> +    }


[*] I may well have seen them before, but my memory is limited and
lossy.