<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Oct 26, 2021 at 5:43 AM Markus Armbruster <<a href="mailto:armbru@redhat.com">armbru@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">John Snow <<a href="mailto:jsnow@redhat.com" target="_blank">jsnow@redhat.com</a>> writes:<br>
<br>
> On Mon, Oct 25, 2021 at 1:26 AM Markus Armbruster <<a href="mailto:armbru@redhat.com" target="_blank">armbru@redhat.com</a>> wrote:<br>
><br>
>> The code to check enumeration value policy can see special feature<br>
>> flag 'deprecated' in QEnumLookup member flags[value].  I want to make<br>
>> feature flag 'unstable' visible there as well, so I can add policy for<br>
>> it.<br>
>><br>
>> Instead of extending flags[], replace it by @special_features (a<br>
>> bitset of QapiSpecialFeature), because that's how special features get<br>
>> passed around elsewhere.<br>
>><br>
>> Signed-off-by: Markus Armbruster <<a href="mailto:armbru@redhat.com" target="_blank">armbru@redhat.com</a>><br>
>> ---<br>
>>  include/qapi/util.h    |  5 +----<br>
>>  qapi/qapi-visit-core.c |  3 ++-<br>
>>  scripts/qapi/types.py  | 22 ++++++++++++----------<br>
>>  3 files changed, 15 insertions(+), 15 deletions(-)<br>
>><br>
>> diff --git a/include/qapi/util.h b/include/qapi/util.h<br>
>> index 7a8d5c7d72..0cc98db9f9 100644<br>
>> --- a/include/qapi/util.h<br>
>> +++ b/include/qapi/util.h<br>
>> @@ -15,12 +15,9 @@ typedef enum {<br>
>>      QAPI_DEPRECATED,<br>
>>  } QapiSpecialFeature;<br>
>><br>
>> -/* QEnumLookup flags */<br>
>> -#define QAPI_ENUM_DEPRECATED 1<br>
>> -<br>
>>  typedef struct QEnumLookup {<br>
>>      const char *const *array;<br>
>> -    const unsigned char *const flags;<br>
>> +    const unsigned char *const special_features;<br>
>>      const int size;<br>
>>  } QEnumLookup;<br>
>><br>
>> diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c<br>
>> index b4a81f1757..5572d90efb 100644<br>
>> --- a/qapi/qapi-visit-core.c<br>
>> +++ b/qapi/qapi-visit-core.c<br>
>> @@ -407,7 +407,8 @@ static bool input_type_enum(Visitor *v, const char<br>
>> *name, int *obj,<br>
>>          return false;<br>
>>      }<br>
>><br>
>> -    if (lookup->flags && (lookup->flags[value] & QAPI_ENUM_DEPRECATED)) {<br>
>> +    if (lookup->special_features<br>
>> +        && (lookup->special_features[value] & QAPI_DEPRECATED)) {<br>
>>          switch (v->compat_policy.deprecated_input) {<br>
>>          case COMPAT_POLICY_INPUT_ACCEPT:<br>
>>              break;<br>
>> diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py<br>
>> index ab2441adc9..3013329c24 100644<br>
>> --- a/scripts/qapi/types.py<br>
>> +++ b/scripts/qapi/types.py<br>
>> @@ -16,7 +16,7 @@<br>
>>  from typing import List, Optional<br>
>><br>
>>  from .common import c_enum_const, c_name, mcgen<br>
>> -from .gen import QAPISchemaModularCVisitor, ifcontext<br>
>> +from .gen import QAPISchemaModularCVisitor, gen_special_features,<br>
>> ifcontext<br>
>>  from .schema import (<br>
>>      QAPISchema,<br>
>>      QAPISchemaEnumMember,<br>
>> @@ -39,7 +39,7 @@ def gen_enum_lookup(name: str,<br>
>>                      members: List[QAPISchemaEnumMember],<br>
>>                      prefix: Optional[str] = None) -> str:<br>
>>      max_index = c_enum_const(name, '_MAX', prefix)<br>
>> -    flags = ''<br>
>> +    feats = ''<br>
>>      ret = mcgen('''<br>
>><br>
>>  const QEnumLookup %(c_name)s_lookup = {<br>
>> @@ -54,19 +54,21 @@ def gen_enum_lookup(name: str,<br>
>>  ''',<br>
>>                       index=index, name=<a href="http://memb.name" rel="noreferrer" target="_blank">memb.name</a>)<br>
>>          ret += memb.ifcond.gen_endif()<br>
>> -        if 'deprecated' in (<a href="http://f.name" rel="noreferrer" target="_blank">f.name</a> for f in memb.features):<br>
>> -            flags += mcgen('''<br>
>> -        [%(index)s] = QAPI_ENUM_DEPRECATED,<br>
>> -''',<br>
>> -                           index=index)<br>
>><br>
>> -    if flags:<br>
>> +        special_features = gen_special_features(memb.features)<br>
>> +        if special_features != '0':<br>
>><br>
><br>
> Though, I have to admit the common reoccurrence of this pattern suggests to<br>
> me that gen_special_features really ought to be returning something false-y<br>
> in these cases. Something about testing for the empty case with something<br>
> that represents, but isn't empty, gives me a brief pause.<br>
><br>
> Not willing to wage war over it.<br>
<br>
I habitually start stupid, and when stupid confuses or annoys me later,<br>
I smarten it up some.<br>
<br>
Let's see how this instance of "stupid" ages, okay?<br>
<br></blockquote></div><div class="gmail_quote"><br></div><div class="gmail_quote">"I see what you're saying, but mehhhhh" is a relatable feeling ;)<br></div><div class="gmail_quote"> <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
>> +            feats += mcgen('''<br>
>> +        [%(index)s] = %(special_features)s,<br>
>> +''',<br>
>> +                           index=index, special_features=special_features)<br>
>> +<br>
>> +    if feats:<br>
>>          ret += mcgen('''<br>
>>      },<br>
>> -    .flags = (const unsigned char[%(max_index)s]) {<br>
>> +    .special_features = (const unsigned char[%(max_index)s]) {<br>
>>  ''',<br>
>>                       max_index=max_index)<br>
>> -        ret += flags<br>
>> +        ret += feats<br>
>><br>
>>      ret += mcgen('''<br>
>>      },<br>
>> --<br>
>> 2.31.1<br>
>><br>
>><br>
> Python bits: Acked-by: John Snow <<a href="mailto:jsnow@redhat.com" target="_blank">jsnow@redhat.com</a>><br>
<br>
Thanks!<br>
<br>
</blockquote></div></div>