| Message ID | 1397628281-11946-1-git-send-email-famz@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 04/16/2014 12:04 AM, Fam Zheng wrote: > In command definition, 'default' is now parsed as a dict of default > values. Only optional parameters will have effect in generated code. Can you make the python code explicitly error out for a default supplied for a parameter not marked with * in data? > > 'str' and 'int' are supported, both need single quote in the schema. In > generated code, 'str' will be converted to g_strdup'ed pointer, 'int' > will be identically set. > > E.g. > > { 'command': 'block-commit', > 'data': { 'device': 'str', '*base': 'str', 'top': 'str', > '*speed': 'int' }, > 'default': {'base': 'earthquake', 'speed': '100' } } 'data' is plural, 'default' is singular; should it be named 'defaults'? > > will generate > > int qmp_marshal_input_block_commit(Monitor *mon, const QDict *qdict, QObject **ret) > { > ... > bool has_base = true; > char * base = g_strdup("earthquake"); Any reason the generator can't be fixed to avoid the space after '*'? > ... > bool has_speed = true; > int64_t speed = 100; Looks like it would work. But why not allow a JSON integer for an integer default: 'data': { '*base': 'str', '*speed', 'int' }, 'default': { 'base': 'earthquake', 'speed': 100 }, I'm not sure if that adds or reduces complexity; by using JSON types as the default, you have to add code in the generator to ensure correct type usage. But it also opens the possibility of providing a default for a struct, rather than limiting to just strings and integers. > > Signed-off-by: Fam Zheng <famz@redhat.com> > --- > scripts/qapi-commands.py | 24 +++++++++++++++++------- > scripts/qapi.py | 8 ++++++++ > 2 files changed, 25 insertions(+), 7 deletions(-) Missing documentation and tests. Docs to docs/qapi-code-gen.txt.
On Fri, Apr 18, 2014 at 03:15:09PM -0600, Eric Blake wrote: > On 04/16/2014 12:04 AM, Fam Zheng wrote: > > In command definition, 'default' is now parsed as a dict of default > > values. Only optional parameters will have effect in generated code. > > Can you make the python code explicitly error out for a default supplied > for a parameter not marked with * in data? > > > > > 'str' and 'int' are supported, both need single quote in the schema. In > > generated code, 'str' will be converted to g_strdup'ed pointer, 'int' > > will be identically set. > > > > E.g. > > > > { 'command': 'block-commit', > > 'data': { 'device': 'str', '*base': 'str', 'top': 'str', > > '*speed': 'int' }, > > 'default': {'base': 'earthquake', 'speed': '100' } } > > 'data' is plural, 'default' is singular; should it be named 'defaults'? > > > > > will generate > > > > int qmp_marshal_input_block_commit(Monitor *mon, const QDict *qdict, QObject **ret) > > { > > ... > > bool has_base = true; > > char * base = g_strdup("earthquake"); > > Any reason the generator can't be fixed to avoid the space after '*'? I knew the reason :-) I will post a patch later. > > ... > > bool has_speed = true; > > int64_t speed = 100; > > Looks like it would work. But why not allow a JSON integer for an > integer default: > > 'data': { '*base': 'str', '*speed', 'int' }, > 'default': { 'base': 'earthquake', 'speed': 100 }, Currently we set the default values insider qmp_COMMAND(), then we need to remove it, or make sure the defule value is same as in qapi-schema.json Do we want to just support simple type? or more complex like: 'data': { '*value': 'ENUM_NAME', .... } 'default': { 'value': 'ENUM_NAME_E1', ... } Or union, type, etc > I'm not sure if that adds or reduces complexity; by using JSON types as > the default, you have to add code in the generator to ensure correct > type usage. But it also opens the possibility of providing a default > for a struct, rather than limiting to just strings and integers. > > > > > Signed-off-by: Fam Zheng <famz@redhat.com> > > --- > > scripts/qapi-commands.py | 24 +++++++++++++++++------- > > scripts/qapi.py | 8 ++++++++ > > 2 files changed, 25 insertions(+), 7 deletions(-) > > Missing documentation and tests. Docs to docs/qapi-code-gen.txt.
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py index 9734ab0..be6eea4 100644 --- a/scripts/qapi-commands.py +++ b/scripts/qapi-commands.py @@ -83,16 +83,23 @@ Visitor *v; return ret.rstrip() -def gen_visitor_input_vars_decl(args): +def gen_visitor_input_vars_decl(args, default): ret = "" push_indent() for argname, argtype, optional, structured in parse_args(args): if optional: ret += mcgen(''' -bool has_%(argname)s = false; +bool has_%(argname)s = %(has_val)s; ''', - argname=c_var(argname)) - if c_type(argtype).endswith("*"): + argname=c_var(argname), + has_val="true" if default.get(argname) else "false") + if optional and default.get(argname): + ret += mcgen(''' +%(argtype)s %(argname)s = %(argval)s; +''', + argname=c_var(argname), argtype=c_type(argtype), + argval=c_val(argtype, default[argname])) + elif c_type(argtype).endswith("*"): ret += mcgen(''' %(argtype)s %(argname)s = NULL; ''', @@ -194,7 +201,7 @@ def gen_marshal_input_decl(name, args, ret_type, middle_mode): -def gen_marshal_input(name, args, ret_type, middle_mode): +def gen_marshal_input(name, args, ret_type, middle_mode, default): hdr = gen_marshal_input_decl(name, args, ret_type, middle_mode) ret = mcgen(''' @@ -229,7 +236,7 @@ def gen_marshal_input(name, args, ret_type, middle_mode): ''', visitor_input_containers_decl=gen_visitor_input_containers_decl(args), - visitor_input_vars_decl=gen_visitor_input_vars_decl(args), + visitor_input_vars_decl=gen_visitor_input_vars_decl(args, default), visitor_input_block=gen_visitor_input_block(args, "QOBJECT(args)")) else: ret += mcgen(''' @@ -434,9 +441,12 @@ if dispatch_type == "sync": for cmd in commands: arglist = [] + default = {} ret_type = None if cmd.has_key('data'): arglist = cmd['data'] + if cmd.has_key('default'): + default = cmd['default'] if cmd.has_key('returns'): ret_type = cmd['returns'] ret = generate_command_decl(cmd['command'], arglist, ret_type) + "\n" @@ -448,7 +458,7 @@ if dispatch_type == "sync": if middle_mode: fdecl.write('%s;\n' % gen_marshal_input_decl(cmd['command'], arglist, ret_type, middle_mode)) - ret = gen_marshal_input(cmd['command'], arglist, ret_type, middle_mode) + "\n" + ret = gen_marshal_input(cmd['command'], arglist, ret_type, middle_mode, default) + "\n" fdef.write(ret) fdecl.write("\n#endif\n"); diff --git a/scripts/qapi.py b/scripts/qapi.py index b474c39..f12866a 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -421,6 +421,14 @@ def find_enum(name): def is_enum(name): return find_enum(name) != None +def c_val(t, val): + if t == 'str': + return 'g_strdup("%s")' % val + elif t == 'int': + return val + else: + assert False, "Unknown type: %s" % t + def c_type(name): if name == 'str': return 'char *'
In command definition, 'default' is now parsed as a dict of default values. Only optional parameters will have effect in generated code. 'str' and 'int' are supported, both need single quote in the schema. In generated code, 'str' will be converted to g_strdup'ed pointer, 'int' will be identically set. E.g. { 'command': 'block-commit', 'data': { 'device': 'str', '*base': 'str', 'top': 'str', '*speed': 'int' }, 'default': {'base': 'earthquake', 'speed': '100' } } will generate int qmp_marshal_input_block_commit(Monitor *mon, const QDict *qdict, QObject **ret) { ... bool has_base = true; char * base = g_strdup("earthquake"); ... bool has_speed = true; int64_t speed = 100; Signed-off-by: Fam Zheng <famz@redhat.com> --- scripts/qapi-commands.py | 24 +++++++++++++++++------- scripts/qapi.py | 8 ++++++++ 2 files changed, 25 insertions(+), 7 deletions(-)