* [Qemu-devel] [PATCH v5 0/3] qapi: fix coding style in generated code @ 2014-05-22 12:41 Amos Kong 2014-05-22 12:41 ` [Qemu-devel] [PATCH v5 1/3] qapi: fix coding style in parameters list Amos Kong ` (2 more replies) 0 siblings, 3 replies; 10+ messages in thread From: Amos Kong @ 2014-05-22 12:41 UTC (permalink / raw) To: qemu-devel; +Cc: pbonzini, lcapitulino, mdroth, armbru Not a serious issue, but it's helpful if we can fix it. V2: split change of scripts/qapi-visit.py to a split patch, eat space by using a special char as Markus suggested V3: update commitlog, update special string, fix of adding const replace string by pattern V4: fix pattern to cleanup special string (Paolo) V5: fix string checking bug (Luiz), update commitlog (Eric) add comments for c_type() function Amos Kong (3): qapi: fix coding style in parameters list qapi: add const prefix to 'char *' insider c_type() qapi: Suppress unwanted space between type and identifier scripts/qapi-commands.py | 6 ++---- scripts/qapi-visit.py | 20 ++++++++++---------- scripts/qapi.py | 21 +++++++++++++++------ 3 files changed, 27 insertions(+), 20 deletions(-) -- 1.9.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH v5 1/3] qapi: fix coding style in parameters list 2014-05-22 12:41 [Qemu-devel] [PATCH v5 0/3] qapi: fix coding style in generated code Amos Kong @ 2014-05-22 12:41 ` Amos Kong 2014-06-02 15:45 ` Markus Armbruster 2014-05-22 12:41 ` [Qemu-devel] [PATCH v5 2/3] qapi: add const prefix to 'char *' insider c_type() Amos Kong 2014-05-22 12:41 ` [Qemu-devel] [PATCH v5 3/3] qapi: Suppress unwanted space between type and identifier Amos Kong 2 siblings, 1 reply; 10+ messages in thread From: Amos Kong @ 2014-05-22 12:41 UTC (permalink / raw) To: qemu-devel; +Cc: pbonzini, lcapitulino, mdroth, armbru A space after * when declaring a pointer type is redundant. Signed-off-by: Amos Kong <akong@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> --- scripts/qapi-visit.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index 06a79f1..c129697 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -77,7 +77,7 @@ static void visit_type_%(full_name)s_field_%(c_name)s(Visitor *m, %(name)s **obj ret += mcgen(''' -static void visit_type_%(full_name)s_fields(Visitor *m, %(name)s ** obj, Error **errp) +static void visit_type_%(full_name)s_fields(Visitor *m, %(name)s **obj, Error **errp) { Error *err = NULL; ''', @@ -186,7 +186,7 @@ def generate_visit_struct(expr): ret += mcgen(''' -void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **errp) +void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error **errp) { ''', name=name) @@ -201,7 +201,7 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error ** def generate_visit_list(name, members): return mcgen(''' -void visit_type_%(name)sList(Visitor *m, %(name)sList ** obj, const char *name, Error **errp) +void visit_type_%(name)sList(Visitor *m, %(name)sList **obj, const char *name, Error **errp) { Error *err = NULL; GenericList *i, **prev; @@ -230,7 +230,7 @@ out: def generate_visit_enum(name, members): return mcgen(''' -void visit_type_%(name)s(Visitor *m, %(name)s * obj, const char *name, Error **errp) +void visit_type_%(name)s(Visitor *m, %(name)s *obj, const char *name, Error **errp) { visit_type_enum(m, (int *)obj, %(name)s_lookup, "%(name)s", name, errp); } @@ -240,7 +240,7 @@ void visit_type_%(name)s(Visitor *m, %(name)s * obj, const char *name, Error **e def generate_visit_anon_union(name, members): ret = mcgen(''' -void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **errp) +void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error **errp) { Error *err = NULL; @@ -327,7 +327,7 @@ def generate_visit_union(expr): ret += mcgen(''' -void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **errp) +void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error **errp) { Error *err = NULL; @@ -399,13 +399,13 @@ def generate_declaration(name, members, genlist=True, builtin_type=False): if not builtin_type: ret += mcgen(''' -void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **errp); +void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error **errp); ''', name=name) if genlist: ret += mcgen(''' -void visit_type_%(name)sList(Visitor *m, %(name)sList ** obj, const char *name, Error **errp); +void visit_type_%(name)sList(Visitor *m, %(name)sList **obj, const char *name, Error **errp); ''', name=name) @@ -415,7 +415,7 @@ def generate_enum_declaration(name, members, genlist=True): ret = "" if genlist: ret += mcgen(''' -void visit_type_%(name)sList(Visitor *m, %(name)sList ** obj, const char *name, Error **errp); +void visit_type_%(name)sList(Visitor *m, %(name)sList **obj, const char *name, Error **errp); ''', name=name) @@ -424,7 +424,7 @@ void visit_type_%(name)sList(Visitor *m, %(name)sList ** obj, const char *name, def generate_decl_enum(name, members, genlist=True): return mcgen(''' -void visit_type_%(name)s(Visitor *m, %(name)s * obj, const char *name, Error **errp); +void visit_type_%(name)s(Visitor *m, %(name)s *obj, const char *name, Error **errp); ''', name=name) -- 1.9.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v5 1/3] qapi: fix coding style in parameters list 2014-05-22 12:41 ` [Qemu-devel] [PATCH v5 1/3] qapi: fix coding style in parameters list Amos Kong @ 2014-06-02 15:45 ` Markus Armbruster 0 siblings, 0 replies; 10+ messages in thread From: Markus Armbruster @ 2014-06-02 15:45 UTC (permalink / raw) To: Amos Kong; +Cc: mdroth, pbonzini, qemu-devel, lcapitulino Amos Kong <akong@redhat.com> writes: > A space after * when declaring a pointer type is redundant. That's not why we clean it up. We clean it up because it's not our style. PATCH 3/3's commit message explains it nicely. > Signed-off-by: Amos Kong <akong@redhat.com> > Reviewed-by: Eric Blake <eblake@redhat.com> > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> > Reviewed-by: Markus Armbruster <armbru@redhat.com> My R-by stands, of course. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH v5 2/3] qapi: add const prefix to 'char *' insider c_type() 2014-05-22 12:41 [Qemu-devel] [PATCH v5 0/3] qapi: fix coding style in generated code Amos Kong 2014-05-22 12:41 ` [Qemu-devel] [PATCH v5 1/3] qapi: fix coding style in parameters list Amos Kong @ 2014-05-22 12:41 ` Amos Kong 2014-05-22 12:41 ` [Qemu-devel] [PATCH v5 3/3] qapi: Suppress unwanted space between type and identifier Amos Kong 2 siblings, 0 replies; 10+ messages in thread From: Amos Kong @ 2014-05-22 12:41 UTC (permalink / raw) To: qemu-devel; +Cc: pbonzini, lcapitulino, mdroth, armbru It's ugly to add const prefix for parameter type by an if statement outside c_type(). This patch adds a parameter to do it. Signed-off-by: Amos Kong <akong@redhat.com> Suggested-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> --- scripts/qapi-commands.py | 4 +--- scripts/qapi.py | 4 +++- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py index 7d93d01..34f200a 100644 --- a/scripts/qapi-commands.py +++ b/scripts/qapi-commands.py @@ -29,9 +29,7 @@ def type_visitor(name): def generate_command_decl(name, args, ret_type): arglist="" for argname, argtype, optional, structured in parse_args(args): - argtype = c_type(argtype) - if argtype == "char *": - argtype = "const char *" + argtype = c_type(argtype, is_param=True) if optional: arglist += "bool has_%s, " % c_var(argname) arglist += "%s %s, " % (argtype, c_var(argname)) diff --git a/scripts/qapi.py b/scripts/qapi.py index 86e9608..dc690bb 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -470,8 +470,10 @@ def find_enum(name): def is_enum(name): return find_enum(name) != None -def c_type(name): +def c_type(name, is_param=False): if name == 'str': + if is_param: + return 'const char *' return 'char *' elif name == 'int': return 'int64_t' -- 1.9.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH v5 3/3] qapi: Suppress unwanted space between type and identifier 2014-05-22 12:41 [Qemu-devel] [PATCH v5 0/3] qapi: fix coding style in generated code Amos Kong 2014-05-22 12:41 ` [Qemu-devel] [PATCH v5 1/3] qapi: fix coding style in parameters list Amos Kong 2014-05-22 12:41 ` [Qemu-devel] [PATCH v5 2/3] qapi: add const prefix to 'char *' insider c_type() Amos Kong @ 2014-05-22 12:41 ` Amos Kong 2014-05-22 16:49 ` Eric Blake 2014-06-02 15:43 ` Markus Armbruster 2 siblings, 2 replies; 10+ messages in thread From: Amos Kong @ 2014-05-22 12:41 UTC (permalink / raw) To: qemu-devel; +Cc: pbonzini, lcapitulino, mdroth, armbru We always generate a space between type and identifier in parameter and variable declarations, even when idiomatic C style doesn't have a space there. Suppress it. Signed-off-by: Amos Kong <akong@redhat.com> --- scripts/qapi-commands.py | 2 +- scripts/qapi.py | 19 +++++++++++++------ 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py index 34f200a..4b49735 100644 --- a/scripts/qapi-commands.py +++ b/scripts/qapi-commands.py @@ -102,7 +102,7 @@ def gen_visitor_input_vars_decl(args): bool has_%(argname)s = false; ''', argname=c_var(argname)) - if c_type(argtype).endswith("*"): + if c_type(argtype).endswith("*" + eatspace): ret += mcgen(''' %(argtype)s %(argname)s = NULL; ''', diff --git a/scripts/qapi.py b/scripts/qapi.py index dc690bb..562d560 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -470,11 +470,17 @@ def find_enum(name): def is_enum(name): return find_enum(name) != None +eatspace = '\033EATSPACE.' + +# A special suffix is added in c_type() for pointer types, and it's +# stripped in mcgen(). So please notice this when you check the return +# value of c_type() outside mcgen(). def c_type(name, is_param=False): if name == 'str': if is_param: - return 'const char *' - return 'char *' + return 'const char *' + eatspace + return 'char *' + eatspace + elif name == 'int': return 'int64_t' elif (name == 'int8' or name == 'int16' or name == 'int32' or @@ -488,15 +494,15 @@ def c_type(name, is_param=False): elif name == 'number': return 'double' elif type(name) == list: - return '%s *' % c_list_type(name[0]) + return '%s *%s' % (c_list_type(name[0]), eatspace) elif is_enum(name): return name elif name == None or len(name) == 0: return 'void' elif name == name.upper(): - return '%sEvent *' % camel_case(name) + return '%sEvent *%s' % (camel_case(name), eatspace) else: - return '%s *' % name + return '%s *%s' % (name, eatspace) def genindent(count): ret = "" @@ -521,7 +527,8 @@ def cgen(code, **kwds): return '\n'.join(lines) % kwds + '\n' def mcgen(code, **kwds): - return cgen('\n'.join(code.split('\n')[1:-1]), **kwds) + raw = cgen('\n'.join(code.split('\n')[1:-1]), **kwds) + return re.sub(re.escape(eatspace) + ' *', '', raw) def basename(filename): return filename.split("/")[-1] -- 1.9.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v5 3/3] qapi: Suppress unwanted space between type and identifier 2014-05-22 12:41 ` [Qemu-devel] [PATCH v5 3/3] qapi: Suppress unwanted space between type and identifier Amos Kong @ 2014-05-22 16:49 ` Eric Blake 2014-06-02 15:43 ` Markus Armbruster 1 sibling, 0 replies; 10+ messages in thread From: Eric Blake @ 2014-05-22 16:49 UTC (permalink / raw) To: Amos Kong, qemu-devel; +Cc: pbonzini, mdroth, armbru, lcapitulino [-- Attachment #1: Type: text/plain, Size: 577 bytes --] On 05/22/2014 06:41 AM, Amos Kong wrote: > We always generate a space between type and identifier in parameter > and variable declarations, even when idiomatic C style doesn't have > a space there. Suppress it. > > Signed-off-by: Amos Kong <akong@redhat.com> > --- > scripts/qapi-commands.py | 2 +- > scripts/qapi.py | 19 +++++++++++++------ > 2 files changed, 14 insertions(+), 7 deletions(-) Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 604 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v5 3/3] qapi: Suppress unwanted space between type and identifier 2014-05-22 12:41 ` [Qemu-devel] [PATCH v5 3/3] qapi: Suppress unwanted space between type and identifier Amos Kong 2014-05-22 16:49 ` Eric Blake @ 2014-06-02 15:43 ` Markus Armbruster 2014-06-09 19:09 ` Luiz Capitulino 2014-06-10 5:56 ` Amos Kong 1 sibling, 2 replies; 10+ messages in thread From: Markus Armbruster @ 2014-06-02 15:43 UTC (permalink / raw) To: Amos Kong; +Cc: mdroth, pbonzini, qemu-devel, lcapitulino Amos Kong <akong@redhat.com> writes: > We always generate a space between type and identifier in parameter > and variable declarations, even when idiomatic C style doesn't have > a space there. Suppress it. > > Signed-off-by: Amos Kong <akong@redhat.com> > --- > scripts/qapi-commands.py | 2 +- > scripts/qapi.py | 19 +++++++++++++------ > 2 files changed, 14 insertions(+), 7 deletions(-) > > diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py > index 34f200a..4b49735 100644 > --- a/scripts/qapi-commands.py > +++ b/scripts/qapi-commands.py > @@ -102,7 +102,7 @@ def gen_visitor_input_vars_decl(args): > bool has_%(argname)s = false; > ''', > argname=c_var(argname)) > - if c_type(argtype).endswith("*"): > + if c_type(argtype).endswith("*" + eatspace): > ret += mcgen(''' > %(argtype)s %(argname)s = NULL; > ''', Ugly. A function is_c_ptr(argtype) would be cleaner. There's similar code in gen_marshal_input(): if ret_type: if c_type(ret_type).endswith("*"): retval = " %s retval = NULL;" % c_type(ret_type) else: retval = " %s retval;" % c_type(ret_type) ret += mcgen(''' %(retval)s ''', retval=retval) Don't you have to patch that, too? > diff --git a/scripts/qapi.py b/scripts/qapi.py > index dc690bb..562d560 100644 > --- a/scripts/qapi.py > +++ b/scripts/qapi.py > @@ -470,11 +470,17 @@ def find_enum(name): > def is_enum(name): > return find_enum(name) != None > > +eatspace = '\033EATSPACE.' > + > +# A special suffix is added in c_type() for pointer types, and it's > +# stripped in mcgen(). So please notice this when you check the return > +# value of c_type() outside mcgen(). > def c_type(name, is_param=False): > if name == 'str': > if is_param: > - return 'const char *' > - return 'char *' > + return 'const char *' + eatspace > + return 'char *' + eatspace > + > elif name == 'int': > return 'int64_t' > elif (name == 'int8' or name == 'int16' or name == 'int32' or > @@ -488,15 +494,15 @@ def c_type(name, is_param=False): > elif name == 'number': > return 'double' > elif type(name) == list: > - return '%s *' % c_list_type(name[0]) > + return '%s *%s' % (c_list_type(name[0]), eatspace) > elif is_enum(name): > return name > elif name == None or len(name) == 0: > return 'void' > elif name == name.upper(): > - return '%sEvent *' % camel_case(name) > + return '%sEvent *%s' % (camel_case(name), eatspace) > else: > - return '%s *' % name > + return '%s *%s' % (name, eatspace) > > def genindent(count): > ret = "" > @@ -521,7 +527,8 @@ def cgen(code, **kwds): > return '\n'.join(lines) % kwds + '\n' > > def mcgen(code, **kwds): > - return cgen('\n'.join(code.split('\n')[1:-1]), **kwds) > + raw = cgen('\n'.join(code.split('\n')[1:-1]), **kwds) > + return re.sub(re.escape(eatspace) + ' *', '', raw) > > def basename(filename): > return filename.split("/")[-1] Rest looks good. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v5 3/3] qapi: Suppress unwanted space between type and identifier 2014-06-02 15:43 ` Markus Armbruster @ 2014-06-09 19:09 ` Luiz Capitulino 2014-06-10 5:56 ` Amos Kong 1 sibling, 0 replies; 10+ messages in thread From: Luiz Capitulino @ 2014-06-09 19:09 UTC (permalink / raw) To: Markus Armbruster; +Cc: pbonzini, Amos Kong, qemu-devel, mdroth On Mon, 02 Jun 2014 17:43:58 +0200 Markus Armbruster <armbru@redhat.com> wrote: > Amos Kong <akong@redhat.com> writes: > > > We always generate a space between type and identifier in parameter > > and variable declarations, even when idiomatic C style doesn't have > > a space there. Suppress it. > > > > Signed-off-by: Amos Kong <akong@redhat.com> > > --- > > scripts/qapi-commands.py | 2 +- > > scripts/qapi.py | 19 +++++++++++++------ > > 2 files changed, 14 insertions(+), 7 deletions(-) > > > > diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py > > index 34f200a..4b49735 100644 > > --- a/scripts/qapi-commands.py > > +++ b/scripts/qapi-commands.py > > @@ -102,7 +102,7 @@ def gen_visitor_input_vars_decl(args): > > bool has_%(argname)s = false; > > ''', > > argname=c_var(argname)) > > - if c_type(argtype).endswith("*"): > > + if c_type(argtype).endswith("*" + eatspace): > > ret += mcgen(''' > > %(argtype)s %(argname)s = NULL; > > ''', > > Ugly. A function is_c_ptr(argtype) would be cleaner. > > There's similar code in gen_marshal_input(): > > if ret_type: > if c_type(ret_type).endswith("*"): > retval = " %s retval = NULL;" % c_type(ret_type) > else: > retval = " %s retval;" % c_type(ret_type) > ret += mcgen(''' > %(retval)s > ''', > retval=retval) > > Don't you have to patch that, too? Amos, have you answered this comment already? This series is pending on this. > > > diff --git a/scripts/qapi.py b/scripts/qapi.py > > index dc690bb..562d560 100644 > > --- a/scripts/qapi.py > > +++ b/scripts/qapi.py > > @@ -470,11 +470,17 @@ def find_enum(name): > > def is_enum(name): > > return find_enum(name) != None > > > > +eatspace = '\033EATSPACE.' > > + > > +# A special suffix is added in c_type() for pointer types, and it's > > +# stripped in mcgen(). So please notice this when you check the return > > +# value of c_type() outside mcgen(). > > def c_type(name, is_param=False): > > if name == 'str': > > if is_param: > > - return 'const char *' > > - return 'char *' > > + return 'const char *' + eatspace > > + return 'char *' + eatspace > > + > > elif name == 'int': > > return 'int64_t' > > elif (name == 'int8' or name == 'int16' or name == 'int32' or > > @@ -488,15 +494,15 @@ def c_type(name, is_param=False): > > elif name == 'number': > > return 'double' > > elif type(name) == list: > > - return '%s *' % c_list_type(name[0]) > > + return '%s *%s' % (c_list_type(name[0]), eatspace) > > elif is_enum(name): > > return name > > elif name == None or len(name) == 0: > > return 'void' > > elif name == name.upper(): > > - return '%sEvent *' % camel_case(name) > > + return '%sEvent *%s' % (camel_case(name), eatspace) > > else: > > - return '%s *' % name > > + return '%s *%s' % (name, eatspace) > > > > def genindent(count): > > ret = "" > > @@ -521,7 +527,8 @@ def cgen(code, **kwds): > > return '\n'.join(lines) % kwds + '\n' > > > > def mcgen(code, **kwds): > > - return cgen('\n'.join(code.split('\n')[1:-1]), **kwds) > > + raw = cgen('\n'.join(code.split('\n')[1:-1]), **kwds) > > + return re.sub(re.escape(eatspace) + ' *', '', raw) > > > > def basename(filename): > > return filename.split("/")[-1] > > Rest looks good. > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v5 3/3] qapi: Suppress unwanted space between type and identifier 2014-06-02 15:43 ` Markus Armbruster 2014-06-09 19:09 ` Luiz Capitulino @ 2014-06-10 5:56 ` Amos Kong 2014-06-12 6:55 ` Markus Armbruster 1 sibling, 1 reply; 10+ messages in thread From: Amos Kong @ 2014-06-10 5:56 UTC (permalink / raw) To: Markus Armbruster; +Cc: mdroth, pbonzini, qemu-devel, lcapitulino On Mon, Jun 02, 2014 at 05:43:58PM +0200, Markus Armbruster wrote: > Amos Kong <akong@redhat.com> writes: > > > We always generate a space between type and identifier in parameter > > and variable declarations, even when idiomatic C style doesn't have > > a space there. Suppress it. > > > > Signed-off-by: Amos Kong <akong@redhat.com> > > --- > > scripts/qapi-commands.py | 2 +- > > scripts/qapi.py | 19 +++++++++++++------ > > 2 files changed, 14 insertions(+), 7 deletions(-) > > > > diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py > > index 34f200a..4b49735 100644 > > --- a/scripts/qapi-commands.py > > +++ b/scripts/qapi-commands.py > > @@ -102,7 +102,7 @@ def gen_visitor_input_vars_decl(args): > > bool has_%(argname)s = false; > > ''', > > argname=c_var(argname)) > > - if c_type(argtype).endswith("*"): > > + if c_type(argtype).endswith("*" + eatspace): > > ret += mcgen(''' > > %(argtype)s %(argname)s = NULL; > > ''', > > Ugly. A function is_c_ptr(argtype) would be cleaner. def is_c_ptr(argtype): suffix = "*" + eatspace return c_type(argtype).endswith(suffix) > There's similar code in gen_marshal_input(): > > if ret_type: > if c_type(ret_type).endswith("*"): > retval = " %s retval = NULL;" % c_type(ret_type) > else: > retval = " %s retval;" % c_type(ret_type) > ret += mcgen(''' > %(retval)s > ''', > retval=retval) Thanks, I searched by grep in the past, but I still lost one :( [amos@z qemu]$ git grep c_type|grep endswith scripts/qapi-commands.py: if c_type(argtype).endswith("*"): scripts/qapi-commands.py: if c_type(ret_type).endswith("*"): > Don't you have to patch that, too? > > > diff --git a/scripts/qapi.py b/scripts/qapi.py > > index dc690bb..562d560 100644 > > --- a/scripts/qapi.py > > +++ b/scripts/qapi.py > > @@ -470,11 +470,17 @@ def find_enum(name): > > def is_enum(name): > > return find_enum(name) != None > > > > +eatspace = '\033EATSPACE.' > > + > > +# A special suffix is added in c_type() for pointer types, and it's > > +# stripped in mcgen(). So please notice this when you check the return > > +# value of c_type() outside mcgen(). > > def c_type(name, is_param=False): > > if name == 'str': > > if is_param: > > - return 'const char *' > > - return 'char *' > > + return 'const char *' + eatspace > > + return 'char *' + eatspace > > + > > elif name == 'int': > > return 'int64_t' > > elif (name == 'int8' or name == 'int16' or name == 'int32' or > > @@ -488,15 +494,15 @@ def c_type(name, is_param=False): > > elif name == 'number': > > return 'double' > > elif type(name) == list: > > - return '%s *' % c_list_type(name[0]) > > + return '%s *%s' % (c_list_type(name[0]), eatspace) > > elif is_enum(name): > > return name > > elif name == None or len(name) == 0: > > return 'void' > > elif name == name.upper(): > > - return '%sEvent *' % camel_case(name) > > + return '%sEvent *%s' % (camel_case(name), eatspace) > > else: > > - return '%s *' % name > > + return '%s *%s' % (name, eatspace) > > > > def genindent(count): > > ret = "" > > @@ -521,7 +527,8 @@ def cgen(code, **kwds): > > return '\n'.join(lines) % kwds + '\n' > > > > def mcgen(code, **kwds): > > - return cgen('\n'.join(code.split('\n')[1:-1]), **kwds) > > + raw = cgen('\n'.join(code.split('\n')[1:-1]), **kwds) > > + return re.sub(re.escape(eatspace) + ' *', '', raw) > > > > def basename(filename): > > return filename.split("/")[-1] > > Rest looks good. -- Amos. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v5 3/3] qapi: Suppress unwanted space between type and identifier 2014-06-10 5:56 ` Amos Kong @ 2014-06-12 6:55 ` Markus Armbruster 0 siblings, 0 replies; 10+ messages in thread From: Markus Armbruster @ 2014-06-12 6:55 UTC (permalink / raw) To: Amos Kong; +Cc: pbonzini, lcapitulino, mdroth, qemu-devel Amos Kong <akong@redhat.com> writes: > On Mon, Jun 02, 2014 at 05:43:58PM +0200, Markus Armbruster wrote: >> Amos Kong <akong@redhat.com> writes: >> >> > We always generate a space between type and identifier in parameter >> > and variable declarations, even when idiomatic C style doesn't have >> > a space there. Suppress it. >> > >> > Signed-off-by: Amos Kong <akong@redhat.com> >> > --- >> > scripts/qapi-commands.py | 2 +- >> > scripts/qapi.py | 19 +++++++++++++------ >> > 2 files changed, 14 insertions(+), 7 deletions(-) >> > >> > diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py >> > index 34f200a..4b49735 100644 >> > --- a/scripts/qapi-commands.py >> > +++ b/scripts/qapi-commands.py >> > @@ -102,7 +102,7 @@ def gen_visitor_input_vars_decl(args): >> > bool has_%(argname)s = false; >> > ''', >> > argname=c_var(argname)) >> > - if c_type(argtype).endswith("*"): >> > + if c_type(argtype).endswith("*" + eatspace): >> > ret += mcgen(''' >> > %(argtype)s %(argname)s = NULL; >> > ''', >> >> Ugly. A function is_c_ptr(argtype) would be cleaner. > > def is_c_ptr(argtype): > suffix = "*" + eatspace > return c_type(argtype).endswith(suffix) Either that, or a suitable conditional matching (relevant parts of) c_type(). Your choice. >> There's similar code in gen_marshal_input(): >> >> if ret_type: >> if c_type(ret_type).endswith("*"): >> retval = " %s retval = NULL;" % c_type(ret_type) >> else: >> retval = " %s retval;" % c_type(ret_type) >> ret += mcgen(''' >> %(retval)s >> ''', >> retval=retval) > > Thanks, I searched by grep in the past, but I still lost one :( That's why we do reviews :) [...] ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2014-06-12 6:56 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-05-22 12:41 [Qemu-devel] [PATCH v5 0/3] qapi: fix coding style in generated code Amos Kong 2014-05-22 12:41 ` [Qemu-devel] [PATCH v5 1/3] qapi: fix coding style in parameters list Amos Kong 2014-06-02 15:45 ` Markus Armbruster 2014-05-22 12:41 ` [Qemu-devel] [PATCH v5 2/3] qapi: add const prefix to 'char *' insider c_type() Amos Kong 2014-05-22 12:41 ` [Qemu-devel] [PATCH v5 3/3] qapi: Suppress unwanted space between type and identifier Amos Kong 2014-05-22 16:49 ` Eric Blake 2014-06-02 15:43 ` Markus Armbruster 2014-06-09 19:09 ` Luiz Capitulino 2014-06-10 5:56 ` Amos Kong 2014-06-12 6:55 ` Markus Armbruster
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.