Re: [Qemu-devel] [PATCH 2/2] Add dynamic generation of module_block.h

[Marc Marí]

On Thu, 27 Aug 2015 10:23:32 +0100
"Daniel P. Berrange" <address@hidden> wrote:

> On Mon, Aug 17, 2015 at 10:09:35AM +0200, Marc Marí wrote:
> > To simplify the addition of new block modules, add a script that
> > generates include/qemu/module_block.h automatically from the
> > modules' source code.
> > 
> > This script assumes that the QEMU coding style rules are followed.
> > 
> > Signed-off-by: Marc Marí <address@hidden>
> > ---
> >  .gitignore                      |   1 +
> >  Makefile                        |  10 ++-
> >  scripts/modules/module_block.py | 132
> > ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 140
> > insertions(+), 3 deletions(-) create mode 100755
> > scripts/modules/module_block.py
> 
> I'd expect to see module_block.h deleted in this commit, otherwise
> you're re-generating a file stored in git each time someone runs
> make.
> 
> > diff --git a/scripts/modules/module_block.py
> > b/scripts/modules/module_block.py new file mode 100755
> > index 0000000..a9a9412
> > --- /dev/null
> > +++ b/scripts/modules/module_block.py
> > @@ -0,0 +1,132 @@
> > +#!/usr/bin/python
> > +#
> > +# Module information generator
> > +#
> > +# Copyright Red Hat, Inc. 2015
> > +#
> > +# Authors:
> > +#  Marc Mari <address@hidden>
> > +#
> > +# This work is licensed under the terms of the GNU GPL, version 2.
> > +# See the COPYING file in the top-level directory.
> > +
> > +import sys
> > +import os
> > +
> > +def get_string_struct(line):
> > +    data = line.split()
> > +
> > +    # data[0] -> struct element name
> > +    # data[1] -> =
> > +    # data[2] -> value
> > +
> > +    return data[2].replace('"', '')[:-1]
> > +
> > +def add_module(fhader, 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},')
> > +
> > +def process_file(fheader, filename):
> > +    # This parser assumes the coding style rules are being followed
> > +    with open(filename, "r") as cfile:
> > +        found_something = False
> > +        found_start = False
> > +        library, _ = os.path.splitext(os.path.basename(filename))
> > +        for line in cfile:
> > +            if found_start:
> > +                line = line.replace('\n', '')
> > +                if line.find(".format_name") != -1:
> > +                    format_name = get_string_struct(line)
> > +                elif line.find(".protocol_name") != -1:
> > +                    protocol_name = get_string_struct(line)
> > +                elif line.find(".bdrv_probe") != -1:
> > +                    probe = True
> > +                elif line.find(".bdrv_probe_device") != -1:
> > +                    probe_device = True
> > +                elif line == "};":
> > +                    add_module(fheader, library, format_name,
> > protocol_name,
> > +                                probe, probe_device)
> > +                    found_start = False
> > +            elif line.find("static BlockDriver") != -1:
> > +                found_something = True
> > +                found_start = True
> > +                format_name = ""
> > +                protocol_name = ""
> > +                probe = False
> > +                probe_device = False
> > +
> > +        if not found_something:
> > +            print("No BlockDriver struct found in " + filename +
> > ". \
> > +                    Is this really a module?")
> > +            sys.exit(1)
> 
> 
> Errors ought to go to sys.stderr rather than stdout.
> 
> 
> 
> > +
> > +def print_top(fheader):
> > +    fheader.write('''/* AUTOMATICALLY GENERATED, DO NOT MODIFY */
> > +/*
> > + * QEMU Block Module Infrastructure
> > + *
> > + * Copyright Red Hat, Inc. 2015
> > + *
> > + * Authors:
> > + *  Marc Mari       <address@hidden>
> 
> When the file is auto-generated, I'm not sure it is right to claim
> copyright / authorship on it - if anything the copyright comes from
> the files that you're using as the source for auto-generation.

I just looked at other autogenerated files, and copied the template.

Thanks
Marc

> > + *
> > + * This work is licensed under the terms of the GNU GPL, version
> > 2.  See
> > + * the COPYING file in the top-level directory.
> > + *
> > + */
> 
> > +# First argument: output folder
> > +# All other arguments: modules source files (.c)
> > +output_dir = sys.argv[1]
> > +if not os.path.isdir(output_dir):
> > +    print("Folder " + output_dir + " does not exist")
> 
> Again about stderr
> 
> > +    sys.exit(1)
> > +
> > +path = output_dir + 'module_block.h'
> > +
> > +with open(path, 'w') as fheader:
> > +    print_top(fheader)
> > +
> > +    for filename in sys.argv[2:]:
> > +        if os.path.isfile(filename):
> > +            process_file(fheader, filename)
> > +        else:
> > +            print("File " + filename + " does not exist.")
> 
> Again here.
> 
> > +            sys.exit(1)
> > +
> > +    print_bottom(fheader)
> > +
> > +sys.exit(0)
> 
> Regards,
> Daniel