| Message ID | 1368152462-13219-2-git-send-email-mdroth@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
On Thu, May 09, 2013 at 09:20:53PM -0500, Michael Roth wrote: > Teach type generators about native types so they can generate the > appropriate linked list types. > > Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com> > --- > scripts/qapi-types.py | 43 ++++++++++++++++++++++++++++++++++++++++--- > scripts/qapi.py | 21 +++++++++++++++++++++ > 2 files changed, 61 insertions(+), 3 deletions(-) > > diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py > index 9e19920..96cb26d 100644 > --- a/scripts/qapi-types.py > +++ b/scripts/qapi-types.py > @@ -16,7 +16,18 @@ import os > import getopt > import errno <snip> > diff --git a/scripts/qapi.py b/scripts/qapi.py > index afc5f32..0ac8c2b 100644 > --- a/scripts/qapi.py > +++ b/scripts/qapi.py > @@ -11,6 +11,10 @@ > > from ordereddict import OrderedDict > > +builtin_types = [ > + 'str', 'int', 'number', 'bool' Will we add size, int8, uint32, ... in future when they are needed? > +] > +
On Fri, May 10, 2013 at 11:04:22AM +0800, Amos Kong wrote: > On Thu, May 09, 2013 at 09:20:53PM -0500, Michael Roth wrote: > > Teach type generators about native types so they can generate the > > appropriate linked list types. > > > > Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com> > > --- > > scripts/qapi-types.py | 43 ++++++++++++++++++++++++++++++++++++++++--- > > scripts/qapi.py | 21 +++++++++++++++++++++ > > 2 files changed, 61 insertions(+), 3 deletions(-) > > > > diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py > > index 9e19920..96cb26d 100644 > > --- a/scripts/qapi-types.py > > +++ b/scripts/qapi-types.py > > @@ -16,7 +16,18 @@ import os > > import getopt > > import errno > > <snip> > > > diff --git a/scripts/qapi.py b/scripts/qapi.py > > index afc5f32..0ac8c2b 100644 > > --- a/scripts/qapi.py > > +++ b/scripts/qapi.py > > @@ -11,6 +11,10 @@ > > > > from ordereddict import OrderedDict > > > > +builtin_types = [ > > + 'str', 'int', 'number', 'bool' > > Will we add size, int8, uint32, ... in future when they are needed? Well, that was the plan, but I hadn't recalled that support for these was already been added to the code generators as part of the Netdev opts-visitor stuff, so I'll go ahead and do a re-spin with those added. > > > +] > > + > > -- > Amos. >
On Thu, 9 May 2013 21:20:53 -0500 Michael Roth <mdroth@linux.vnet.ibm.com> wrote: > Teach type generators about native types so they can generate the > appropriate linked list types. > > Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com> > --- > scripts/qapi-types.py | 43 ++++++++++++++++++++++++++++++++++++++++--- > scripts/qapi.py | 21 +++++++++++++++++++++ > 2 files changed, 61 insertions(+), 3 deletions(-) > > diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py > index 9e19920..96cb26d 100644 > --- a/scripts/qapi-types.py > +++ b/scripts/qapi-types.py > @@ -16,7 +16,18 @@ import os > import getopt > import errno > > -def generate_fwd_struct(name, members): > +def generate_fwd_struct(name, members, builtin_type=False): > + if builtin_type: > + return mcgen(''' > +typedef struct %(name)sList > +{ > + %(type)s value; > + struct %(name)sList *next; > +} %(name)sList; > +''', Sorry for the utterly minor comment, but as you're going to respin please add a newline before ''' so that we get the declarations properly separated when generated. > + type=c_type(name), > + name=name) > + > return mcgen(''' > typedef struct %(name)s %(name)s; > > @@ -164,6 +175,7 @@ void qapi_free_%(type)s(%(c_type)s obj); > > def generate_type_cleanup(name): > ret = mcgen(''' > + > void qapi_free_%(type)s(%(c_type)s obj) > { > QapiDeallocVisitor *md; > @@ -184,8 +196,9 @@ void qapi_free_%(type)s(%(c_type)s obj) > > > try: > - opts, args = getopt.gnu_getopt(sys.argv[1:], "chp:o:", > - ["source", "header", "prefix=", "output-dir="]) > + opts, args = getopt.gnu_getopt(sys.argv[1:], "chbp:o:", > + ["source", "header", "builtins", > + "prefix=", "output-dir="]) > except getopt.GetoptError, err: > print str(err) > sys.exit(1) > @@ -197,6 +210,7 @@ h_file = 'qapi-types.h' > > do_c = False > do_h = False > +do_builtins = False > > for o, a in opts: > if o in ("-p", "--prefix"): > @@ -207,6 +221,8 @@ for o, a in opts: > do_c = True > elif o in ("-h", "--header"): > do_h = True > + elif o in ("-b", "--builtins"): > + do_builtins = True > > if not do_c and not do_h: > do_c = True > @@ -282,6 +298,11 @@ fdecl.write(mcgen(''' > exprs = parse_schema(sys.stdin) > exprs = filter(lambda expr: not expr.has_key('gen'), exprs) > > +fdecl.write(guardstart("QAPI_TYPES_BUILTIN_STRUCT_DECL")) > +for typename in builtin_types: > + fdecl.write(generate_fwd_struct(typename, None, builtin_type=True)) > +fdecl.write(guardend("QAPI_TYPES_BUILTIN_STRUCT_DECL")) > + > for expr in exprs: > ret = "\n" > if expr.has_key('type'): > @@ -298,6 +319,22 @@ for expr in exprs: > continue > fdecl.write(ret) > > +# to avoid header dependency hell, we always generate declarations > +# for built-in types in our header files and simply guard them > +fdecl.write(guardstart("QAPI_TYPES_BUILTIN_CLEANUP_DECL")) > +for typename in builtin_types: > + fdecl.write(generate_type_cleanup_decl(typename + "List")) > +fdecl.write(guardend("QAPI_TYPES_BUILTIN_CLEANUP_DECL")) I'm not sure I got why you're doing this. Is it because you're going to generate them in more .h files? This is a bit ugly :( > + > +# ...this doesn't work for cases where we link in multiple objects that > +# have the functions defined, so we use -b option to provide control > +# over these cases > +if do_builtins: > + fdef.write(guardstart("QAPI_TYPES_BUILTIN_CLEANUP_DEF")) > + for typename in builtin_types: > + fdef.write(generate_type_cleanup(typename + "List")) > + fdef.write(guardend("QAPI_TYPES_BUILTIN_CLEANUP_DEF")) > + > for expr in exprs: > ret = "\n" > if expr.has_key('type'): > diff --git a/scripts/qapi.py b/scripts/qapi.py > index afc5f32..0ac8c2b 100644 > --- a/scripts/qapi.py > +++ b/scripts/qapi.py > @@ -11,6 +11,10 @@ > > from ordereddict import OrderedDict > > +builtin_types = [ > + 'str', 'int', 'number', 'bool' > +] > + > def tokenize(data): > while len(data): > ch = data[0] > @@ -242,3 +246,20 @@ def guardname(filename): > for substr in [".", " ", "-"]: > guard = guard.replace(substr, "_") > return guard.upper() + '_H' > + > +def guardstart(name): > + return mcgen(''' > + > +#ifndef %(name)s > +#define %(name)s > + > +''', > + name=guardname(name)) > + > +def guardend(name): > + return mcgen(''' > + > +#endif /* %(name)s */ > + > +''', > + name=guardname(name))
On Fri, May 10, 2013 at 10:07:45AM -0400, Luiz Capitulino wrote: > On Thu, 9 May 2013 21:20:53 -0500 > Michael Roth <mdroth@linux.vnet.ibm.com> wrote: > > > Teach type generators about native types so they can generate the > > appropriate linked list types. > > > > Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com> > > --- > > scripts/qapi-types.py | 43 ++++++++++++++++++++++++++++++++++++++++--- > > scripts/qapi.py | 21 +++++++++++++++++++++ > > 2 files changed, 61 insertions(+), 3 deletions(-) > > > > diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py > > index 9e19920..96cb26d 100644 > > --- a/scripts/qapi-types.py > > +++ b/scripts/qapi-types.py > > @@ -16,7 +16,18 @@ import os > > import getopt > > import errno > > > > -def generate_fwd_struct(name, members): > > +def generate_fwd_struct(name, members, builtin_type=False): > > + if builtin_type: > > + return mcgen(''' > > +typedef struct %(name)sList > > +{ > > + %(type)s value; > > + struct %(name)sList *next; > > +} %(name)sList; > > +''', > > Sorry for the utterly minor comment, but as you're going to respin please > add a newline before ''' so that we get the declarations properly separated > when generated. > > > + type=c_type(name), > > + name=name) > > + > > return mcgen(''' > > typedef struct %(name)s %(name)s; > > > > @@ -164,6 +175,7 @@ void qapi_free_%(type)s(%(c_type)s obj); > > > > def generate_type_cleanup(name): > > ret = mcgen(''' > > + > > void qapi_free_%(type)s(%(c_type)s obj) > > { > > QapiDeallocVisitor *md; > > @@ -184,8 +196,9 @@ void qapi_free_%(type)s(%(c_type)s obj) > > > > > > try: > > - opts, args = getopt.gnu_getopt(sys.argv[1:], "chp:o:", > > - ["source", "header", "prefix=", "output-dir="]) > > + opts, args = getopt.gnu_getopt(sys.argv[1:], "chbp:o:", > > + ["source", "header", "builtins", > > + "prefix=", "output-dir="]) > > except getopt.GetoptError, err: > > print str(err) > > sys.exit(1) > > @@ -197,6 +210,7 @@ h_file = 'qapi-types.h' > > > > do_c = False > > do_h = False > > +do_builtins = False > > > > for o, a in opts: > > if o in ("-p", "--prefix"): > > @@ -207,6 +221,8 @@ for o, a in opts: > > do_c = True > > elif o in ("-h", "--header"): > > do_h = True > > + elif o in ("-b", "--builtins"): > > + do_builtins = True > > > > if not do_c and not do_h: > > do_c = True > > @@ -282,6 +298,11 @@ fdecl.write(mcgen(''' > > exprs = parse_schema(sys.stdin) > > exprs = filter(lambda expr: not expr.has_key('gen'), exprs) > > > > +fdecl.write(guardstart("QAPI_TYPES_BUILTIN_STRUCT_DECL")) > > +for typename in builtin_types: > > + fdecl.write(generate_fwd_struct(typename, None, builtin_type=True)) > > +fdecl.write(guardend("QAPI_TYPES_BUILTIN_STRUCT_DECL")) > > + > > for expr in exprs: > > ret = "\n" > > if expr.has_key('type'): > > @@ -298,6 +319,22 @@ for expr in exprs: > > continue > > fdecl.write(ret) > > > > +# to avoid header dependency hell, we always generate declarations > > +# for built-in types in our header files and simply guard them > > +fdecl.write(guardstart("QAPI_TYPES_BUILTIN_CLEANUP_DECL")) > > +for typename in builtin_types: > > + fdecl.write(generate_type_cleanup_decl(typename + "List")) > > +fdecl.write(guardend("QAPI_TYPES_BUILTIN_CLEANUP_DECL")) > > I'm not sure I got why you're doing this. Is it because you're going to > generate them in more .h files? This is a bit ugly :( > The issue is that things like the types generated from qapi-schema-test.json or qga/qapi-schema.json may end up referencing intList/strList/visit_type_intList/etc, which we'll also have declared for use in the main qapi-schema.json. qapi-schema.json of course can't depend on tests or qga, so we generate the definitions for the builtin types when running the code generators on qapi-schema.json. For everyone else, tests/qga/etc, to be able to use/link against those definitions we need to include the declarations by either #include'ing qapi-types.h/qapi-visit.h etc, or by always declaring them and simply adding a guard. In this case I've taken the latter approach since hard-coding a reference in the code generators to header files created by separate calls to the code generators seemed less modular. qapi-schema-test.json should be capable of generating self-contained code if need be (and the only reason we don't make it self-contained is that test-cases rely on libqemuutil to build, which actually does have a dependency on qapi-schema.json due to qemu-sockets. > > + > > +# ...this doesn't work for cases where we link in multiple objects that > > +# have the functions defined, so we use -b option to provide control > > +# over these cases > > +if do_builtins: > > + fdef.write(guardstart("QAPI_TYPES_BUILTIN_CLEANUP_DEF")) > > + for typename in builtin_types: > > + fdef.write(generate_type_cleanup(typename + "List")) > > + fdef.write(guardend("QAPI_TYPES_BUILTIN_CLEANUP_DEF")) > > + > > for expr in exprs: > > ret = "\n" > > if expr.has_key('type'): > > diff --git a/scripts/qapi.py b/scripts/qapi.py > > index afc5f32..0ac8c2b 100644 > > --- a/scripts/qapi.py > > +++ b/scripts/qapi.py > > @@ -11,6 +11,10 @@ > > > > from ordereddict import OrderedDict > > > > +builtin_types = [ > > + 'str', 'int', 'number', 'bool' > > +] > > + > > def tokenize(data): > > while len(data): > > ch = data[0] > > @@ -242,3 +246,20 @@ def guardname(filename): > > for substr in [".", " ", "-"]: > > guard = guard.replace(substr, "_") > > return guard.upper() + '_H' > > + > > +def guardstart(name): > > + return mcgen(''' > > + > > +#ifndef %(name)s > > +#define %(name)s > > + > > +''', > > + name=guardname(name)) > > + > > +def guardend(name): > > + return mcgen(''' > > + > > +#endif /* %(name)s */ > > + > > +''', > > + name=guardname(name)) >
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py index 9e19920..96cb26d 100644 --- a/scripts/qapi-types.py +++ b/scripts/qapi-types.py @@ -16,7 +16,18 @@ import os import getopt import errno -def generate_fwd_struct(name, members): +def generate_fwd_struct(name, members, builtin_type=False): + if builtin_type: + return mcgen(''' +typedef struct %(name)sList +{ + %(type)s value; + struct %(name)sList *next; +} %(name)sList; +''', + type=c_type(name), + name=name) + return mcgen(''' typedef struct %(name)s %(name)s; @@ -164,6 +175,7 @@ void qapi_free_%(type)s(%(c_type)s obj); def generate_type_cleanup(name): ret = mcgen(''' + void qapi_free_%(type)s(%(c_type)s obj) { QapiDeallocVisitor *md; @@ -184,8 +196,9 @@ void qapi_free_%(type)s(%(c_type)s obj) try: - opts, args = getopt.gnu_getopt(sys.argv[1:], "chp:o:", - ["source", "header", "prefix=", "output-dir="]) + opts, args = getopt.gnu_getopt(sys.argv[1:], "chbp:o:", + ["source", "header", "builtins", + "prefix=", "output-dir="]) except getopt.GetoptError, err: print str(err) sys.exit(1) @@ -197,6 +210,7 @@ h_file = 'qapi-types.h' do_c = False do_h = False +do_builtins = False for o, a in opts: if o in ("-p", "--prefix"): @@ -207,6 +221,8 @@ for o, a in opts: do_c = True elif o in ("-h", "--header"): do_h = True + elif o in ("-b", "--builtins"): + do_builtins = True if not do_c and not do_h: do_c = True @@ -282,6 +298,11 @@ fdecl.write(mcgen(''' exprs = parse_schema(sys.stdin) exprs = filter(lambda expr: not expr.has_key('gen'), exprs) +fdecl.write(guardstart("QAPI_TYPES_BUILTIN_STRUCT_DECL")) +for typename in builtin_types: + fdecl.write(generate_fwd_struct(typename, None, builtin_type=True)) +fdecl.write(guardend("QAPI_TYPES_BUILTIN_STRUCT_DECL")) + for expr in exprs: ret = "\n" if expr.has_key('type'): @@ -298,6 +319,22 @@ for expr in exprs: continue fdecl.write(ret) +# to avoid header dependency hell, we always generate declarations +# for built-in types in our header files and simply guard them +fdecl.write(guardstart("QAPI_TYPES_BUILTIN_CLEANUP_DECL")) +for typename in builtin_types: + fdecl.write(generate_type_cleanup_decl(typename + "List")) +fdecl.write(guardend("QAPI_TYPES_BUILTIN_CLEANUP_DECL")) + +# ...this doesn't work for cases where we link in multiple objects that +# have the functions defined, so we use -b option to provide control +# over these cases +if do_builtins: + fdef.write(guardstart("QAPI_TYPES_BUILTIN_CLEANUP_DEF")) + for typename in builtin_types: + fdef.write(generate_type_cleanup(typename + "List")) + fdef.write(guardend("QAPI_TYPES_BUILTIN_CLEANUP_DEF")) + for expr in exprs: ret = "\n" if expr.has_key('type'): diff --git a/scripts/qapi.py b/scripts/qapi.py index afc5f32..0ac8c2b 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -11,6 +11,10 @@ from ordereddict import OrderedDict +builtin_types = [ + 'str', 'int', 'number', 'bool' +] + def tokenize(data): while len(data): ch = data[0] @@ -242,3 +246,20 @@ def guardname(filename): for substr in [".", " ", "-"]: guard = guard.replace(substr, "_") return guard.upper() + '_H' + +def guardstart(name): + return mcgen(''' + +#ifndef %(name)s +#define %(name)s + +''', + name=guardname(name)) + +def guardend(name): + return mcgen(''' + +#endif /* %(name)s */ + +''', + name=guardname(name))
Teach type generators about native types so they can generate the appropriate linked list types. Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com> --- scripts/qapi-types.py | 43 ++++++++++++++++++++++++++++++++++++++++--- scripts/qapi.py | 21 +++++++++++++++++++++ 2 files changed, 61 insertions(+), 3 deletions(-)