Re: [Qemu-devel] [PATCH v3 02/32] blockdev: Add dynamic generation of module_block.h

[Colin Lord <clord@redhat.com>]

On 07/06/2016 09:17 AM, Max Reitz wrote:
> On 05.07.2016 17:24, Colin Lord wrote:
>> From: Marc Mari <markmb@redhat.com>
>>
>> To simplify the addition of new block modules, add a script that generates
>> module_block.h automatically from the modules' source code.
>>
>> This script assumes that the QEMU coding style rules are followed.
> 
> But the script itself doesn't follow it. :-P
> ("ERROR: code indent should never use tabs", to quote checkpatch.pl)
> 
I guess that is the case.

>>
>> Signed-off-by: Marc Marí <markmb@redhat.com>
>> Signed-off-by: Colin Lord <clord@redhat.com>
>> ---
>>  Makefile                        |   7 +++
>>  scripts/modules/module_block.py | 122 ++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 129 insertions(+)
>>  create mode 100644 scripts/modules/module_block.py
>>
> 
> [...]
> 
>> diff --git a/scripts/modules/module_block.py b/scripts/modules/module_block.py
>> new file mode 100644
>> index 0000000..4075574
>> --- /dev/null
>> +++ b/scripts/modules/module_block.py
>> @@ -0,0 +1,122 @@
> 
> [...]
> 
>> +def add_module(fheader, library, format_name, protocol_name,
>> +                probe, probe_device):
>> +    lines = []
>> +    lines.append('.library_name = "' + library + '",')
>> +    if format_name != "":
>> +        lines.append('.format_name = "' + format_name + '",')
>> +    if protocol_name != "":
>> +        lines.append('.protocol_name = "' + protocol_name + '",')
>> +    if probe:
>> +        lines.append('.has_probe = true,')
>> +    if probe_device:
>> +        lines.append('.has_probe_device = true,')
>> +
>> +    text = '\n\t'.join(lines)
>> +    fheader.write('\n\t{\n\t' + text + '\n\t},')
> 
> I think our normal coding style would be met with:
> 
> text = '\n        '.join(lines)
> fheader.write('\n    {\n        ' + text + '\n    },')
> 
> Rest looks good, so with that fixed (or maybe we don't need to fix it
> because it's just a generated header, I don't know):
> 
Shouldn't be too big a deal to fix.

> Reviewed-by: Max Reitz <mreitz@redhat.com>
>