Re: [PATCH v6 10/14] block-coroutine-wrapper.py: introduce co_wrapper

[Emanuele Giuseppe Esposito <eesposit@redhat.com>]

Am 25/11/2022 um 21:32 schrieb Vladimir Sementsov-Ogievskiy:
> 
>>     class FuncDecl:
>> -    def __init__(self, return_type: str, name: str, args: str) -> None:
>> +    def __init__(self, return_type: str, name: str, args: str,
>> +                 variant: str) -> None:
> 
> I'd prefer mixed: bool parameter instead, to be more strict.
> 
>>           self.return_type = return_type.strip()
>>           self.name = name.strip()
>> +        self.struct_name = snake_to_camel(self.name)
>>           self.args = [ParamDecl(arg.strip()) for arg in args.split(',')]
>> +        self.create_only_co = True
>> +
>> +        if 'mixed' in variant:
>> +            self.create_only_co = False
> 
> hmm, just
> 
>   self.create_only_co = 'mixed' not in variant
> 
> ? And even better with boolean argument.
> 
>> +
>> +        subsystem, subname = self.name.split('_', 1)
>> +        self.co_name = f'{subsystem}_co_{subname}'
>> +
>> +        t = self.args[0].type
>> +        if t == 'BlockDriverState *':
>> +            bs = 'bs'
>> +        elif t == 'BdrvChild *':
>> +            bs = 'child->bs'
>> +        else:
>> +            bs = 'blk_bs(blk)'
>> +        self.bs = bs
>>         def gen_list(self, format: str) -> str:
>>           return ', '.join(format.format_map(arg.__dict__) for arg in
>> self.args)
>> @@ -74,8 +92,9 @@ def gen_block(self, format: str) -> str:
>>           return '\n'.join(format.format_map(arg.__dict__) for arg in
>> self.args)
>>     -# Match wrappers declared with a co_wrapper_mixed mark
>> -func_decl_re = re.compile(r'^int\s*co_wrapper_mixed\s*'
>> +# Match wrappers declared with a co_wrapper mark
>> +func_decl_re = re.compile(r'^int\s*co_wrapper'
>> +                          r'(?P<variant>(_[a-z][a-z0-9_]*)?)\s*'
> 
> Why you allow everything here?
> I'd write just
>  
>   (?P<mixed>(_mixed)?)
> 
> or
> 
>   (?P<marker>co_wrapper(_mixed)?)

Ok you couldn't possibly know that, but we are also adding other type of
"variants":
co_wrapper
co_wrapper_mixed
co_wrapper_bdrv_rdlock
co_wrapper_mixed_bdrv_rdlock

Therefore I need to keep variant : str and the regex as it is, but maybe
get rid of the if else condition. I'll change the docstring of course.

If you want to know more, see the thread in
"[PATCH 00/15] Protect the block layer with a rwlock: part 3"

Thank you,
Emanuele