Re: [Qemu-devel] [PATCH] qapi: generate space in c_type() to fix coding style
Amos Kong writes: > On Tue, Apr 22, 2014 at 09:20:41AM +0200, Markus Armbruster wrote: >> Paolo Bonzini writes: >> >> > Il 18/04/2014 23:56, Amos Kong ha scritto: >> >> Currently we always add a space after c_type in mcgen(), there is >> >> some redundant space in generated code. The space isn't needed for >> >> points by the coding style. >> >> >> >> char * value; >> >> ^ >> >> qapi_free_NameInfo(NameInfo * obj) >> >>^ >> >> It's fussy to add checking in each mcgen(), this patch just addes >> >> the necessary space in c_type(), and remove original space in mcgen(). >> > >> > It also makes the generator harder to read to though, and the >> > improvement in the generated code is minor enough that I think the >> > change is overall negative. But I won't oppose the patch if the >> > maintainers are fine with it. > > Hi Markus, > >> The readability hit could be avoided: instead of moving the space from >> mcgen()'s argument to c_type()'s result, add a "hungry character" to >> c_type()'s result, then make it eat space to the right in mcgen(). > > '\b' is used to eat char in left, but what's the "hungry character" to > each char in right? > > When I added a '\b' to c_type()'s return, there is an arror: > error: stray '\10' in program I'm afraid you misunderstood me. Pick a character that cannot occur in valid C code. Make c_type()'s return value end in that character in the cases where you don't want space. Then make mcgen() strip it from its return value along with immediately following space. Questions? [...]
Re: [Qemu-devel] [PATCH] qapi: generate space in c_type() to fix coding style
On Tue, Apr 22, 2014 at 09:20:41AM +0200, Markus Armbruster wrote:
> Paolo Bonzini writes:
>
> > Il 18/04/2014 23:56, Amos Kong ha scritto:
> >> Currently we always add a space after c_type in mcgen(), there is
> >> some redundant space in generated code. The space isn't needed for
> >> points by the coding style.
> >>
> >> char * value;
> >> ^
> >> qapi_free_NameInfo(NameInfo * obj)
> >>^
> >> It's fussy to add checking in each mcgen(), this patch just addes
> >> the necessary space in c_type(), and remove original space in mcgen().
> >
> > It also makes the generator harder to read to though, and the
> > improvement in the generated code is minor enough that I think the
> > change is overall negative. But I won't oppose the patch if the
> > maintainers are fine with it.
Hi Markus,
> The readability hit could be avoided: instead of moving the space from
> mcgen()'s argument to c_type()'s result, add a "hungry character" to
> c_type()'s result, then make it eat space to the right in mcgen().
'\b' is used to eat char in left, but what's the "hungry character" to
each char in right?
When I added a '\b' to c_type()'s return, there is an arror:
error: stray '\10' in program
Another solution:
I tried to use '\b' to eat the redundant space by following code:
| def eat_space(type):
| if type[-1] == '*':
| return '\b'
| return ''
| return mcgen('''
|
| typedef struct %(name)sList
| {
| union {
| %(type)s %(space)svalue;
| uint64_t padding;
| };
| struct %(name)sList *next;
| } %(name)sList;
| ''',
| type=c_type(name),
| space=eat_space(name),
| name=name)
I looks still not good ?
--
Amos.
Re: [Qemu-devel] [PATCH] qapi: generate space in c_type() to fix coding style
Paolo Bonzini writes: > Il 18/04/2014 23:56, Amos Kong ha scritto: >> Currently we always add a space after c_type in mcgen(), there is >> some redundant space in generated code. The space isn't needed for >> points by the coding style. >> >> char * value; >> ^ >> qapi_free_NameInfo(NameInfo * obj) >>^ >> It's fussy to add checking in each mcgen(), this patch just addes >> the necessary space in c_type(), and remove original space in mcgen(). > > It also makes the generator harder to read to though, and the > improvement in the generated code is minor enough that I think the > change is overall negative. But I won't oppose the patch if the > maintainers are fine with it. The readability hit could be avoided: instead of moving the space from mcgen()'s argument to c_type()'s result, add a "hungry character" to c_type()'s result, then make it eat space to the right in mcgen().
Re: [Qemu-devel] [PATCH] qapi: generate space in c_type() to fix coding style
On Sat, Apr 19, 2014 at 03:16:52PM -0600, Eric Blake wrote:
> On 04/18/2014 09:56 PM, Amos Kong wrote:
> > Currently we always add a space after c_type in mcgen(), there is
> > some redundant space in generated code. The space isn't needed for
> > points by the coding style.
>
> Second sentence is awkward; maybe:
>
> Avoiding the space when appropriate makes us match the coding style.
>
> >
> > char * value;
> > ^
> > qapi_free_NameInfo(NameInfo * obj)
> >^
> > It's fussy to add checking in each mcgen(), this patch just addes
>
> s/addes/adds/
>
> > the necessary space in c_type(), and remove original space in mcgen().
>
> s/remove/removes the/
>
> >
> > Signed-off-by: Amos Kong
> > ---
> > scripts/qapi-types.py | 10 +-
> > scripts/qapi-visit.py | 20 ++--
> > scripts/qapi.py | 14 +++---
> > 3 files changed, 22 insertions(+), 22 deletions(-)
>
> > union {
> > -%(type)s value;
> > +%(type)svalue;
>
> Code itself does what it claims. It's a bit harder to read the
> generator without the space, and I might have added a comment to
> c_type() explaining that the output string includes the space except for
> pointer types. I'll let others decide whether to take the patch, but
> I'm comfortable if the commit message is fixed and you want to add:
>
> Reviewed-by: Eric Blake
I queued this patch for long time, I'm also not sure if it's
necessary. I planed to send it out when I saw your recent comment
about this issue.
Michael, what's your opinion?
> --
> Eric Blake eblake redhat com+1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>
--
Amos.
pgpRgE2x6LblZ.pgp
Description: PGP signature
Re: [Qemu-devel] [PATCH] qapi: generate space in c_type() to fix coding style
On 04/18/2014 09:56 PM, Amos Kong wrote:
> Currently we always add a space after c_type in mcgen(), there is
> some redundant space in generated code. The space isn't needed for
> points by the coding style.
Second sentence is awkward; maybe:
Avoiding the space when appropriate makes us match the coding style.
>
> char * value;
> ^
> qapi_free_NameInfo(NameInfo * obj)
>^
> It's fussy to add checking in each mcgen(), this patch just addes
s/addes/adds/
> the necessary space in c_type(), and remove original space in mcgen().
s/remove/removes the/
>
> Signed-off-by: Amos Kong
> ---
> scripts/qapi-types.py | 10 +-
> scripts/qapi-visit.py | 20 ++--
> scripts/qapi.py | 14 +++---
> 3 files changed, 22 insertions(+), 22 deletions(-)
> union {
> -%(type)s value;
> +%(type)svalue;
Code itself does what it claims. It's a bit harder to read the
generator without the space, and I might have added a comment to
c_type() explaining that the output string includes the space except for
pointer types. I'll let others decide whether to take the patch, but
I'm comfortable if the commit message is fixed and you want to add:
Reviewed-by: Eric Blake
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] qapi: generate space in c_type() to fix coding style
Il 18/04/2014 23:56, Amos Kong ha scritto:
Currently we always add a space after c_type in mcgen(), there is
some redundant space in generated code. The space isn't needed for
points by the coding style.
char * value;
^
qapi_free_NameInfo(NameInfo * obj)
^
It's fussy to add checking in each mcgen(), this patch just addes
the necessary space in c_type(), and remove original space in mcgen().
It also makes the generator harder to read to though, and the
improvement in the generated code is minor enough that I think the
change is overall negative. But I won't oppose the patch if the
maintainers are fine with it.
Paolo
Signed-off-by: Amos Kong
---
scripts/qapi-types.py | 10 +-
scripts/qapi-visit.py | 20 ++--
scripts/qapi.py | 14 +++---
3 files changed, 22 insertions(+), 22 deletions(-)
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 10864ef..54a9b89 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -23,7 +23,7 @@ def generate_fwd_struct(name, members, builtin_type=False):
typedef struct %(name)sList
{
union {
-%(type)s value;
+%(type)svalue;
uint64_t padding;
};
struct %(name)sList *next;
@@ -75,7 +75,7 @@ def generate_struct_fields(members):
pop_indent()
else:
ret += mcgen('''
-%(c_type)s %(c_name)s;
+%(c_type)s%(c_name)s;
''',
c_type=c_type(argentry), c_name=c_var(argname))
@@ -219,7 +219,7 @@ struct %(name)s
for key in typeinfo:
ret += mcgen('''
-%(c_type)s %(c_name)s;
+%(c_type)s%(c_name)s;
''',
c_type=c_type(typeinfo[key]),
c_name=c_fun(key))
@@ -251,7 +251,7 @@ extern const int %(name)s_qtypes[];
def generate_type_cleanup_decl(name):
ret = mcgen('''
-void qapi_free_%(type)s(%(c_type)s obj);
+void qapi_free_%(type)s(%(c_type)sobj);
''',
c_type=c_type(name),type=name)
return ret
@@ -259,7 +259,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)
+void qapi_free_%(type)s(%(c_type)sobj)
{
QapiDeallocVisitor *md;
Visitor *v;
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 45ce3a9..8ffed3d 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -38,7 +38,7 @@ def generate_visit_struct_fields(name, field_prefix,
fn_prefix, members, base =
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;
''',
@@ -149,7 +149,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)
@@ -166,7 +166,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)
{
GenericList *i, **prev = (GenericList **)obj;
Error *err = NULL;
@@ -193,7 +193,7 @@ void visit_type_%(name)sList(Visitor *m, %(name)sList **
obj, const char *name,
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);
}
@@ -203,7 +203,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;
@@ -279,7 +279,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;
@@ -367,13 +367,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 vis
[Qemu-devel] [PATCH] qapi: generate space in c_type() to fix coding style
Currently we always add a space after c_type in mcgen(), there is
some redundant space in generated code. The space isn't needed for
points by the coding style.
char * value;
^
qapi_free_NameInfo(NameInfo * obj)
^
It's fussy to add checking in each mcgen(), this patch just addes
the necessary space in c_type(), and remove original space in mcgen().
Signed-off-by: Amos Kong
---
scripts/qapi-types.py | 10 +-
scripts/qapi-visit.py | 20 ++--
scripts/qapi.py | 14 +++---
3 files changed, 22 insertions(+), 22 deletions(-)
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 10864ef..54a9b89 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -23,7 +23,7 @@ def generate_fwd_struct(name, members, builtin_type=False):
typedef struct %(name)sList
{
union {
-%(type)s value;
+%(type)svalue;
uint64_t padding;
};
struct %(name)sList *next;
@@ -75,7 +75,7 @@ def generate_struct_fields(members):
pop_indent()
else:
ret += mcgen('''
-%(c_type)s %(c_name)s;
+%(c_type)s%(c_name)s;
''',
c_type=c_type(argentry), c_name=c_var(argname))
@@ -219,7 +219,7 @@ struct %(name)s
for key in typeinfo:
ret += mcgen('''
-%(c_type)s %(c_name)s;
+%(c_type)s%(c_name)s;
''',
c_type=c_type(typeinfo[key]),
c_name=c_fun(key))
@@ -251,7 +251,7 @@ extern const int %(name)s_qtypes[];
def generate_type_cleanup_decl(name):
ret = mcgen('''
-void qapi_free_%(type)s(%(c_type)s obj);
+void qapi_free_%(type)s(%(c_type)sobj);
''',
c_type=c_type(name),type=name)
return ret
@@ -259,7 +259,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)
+void qapi_free_%(type)s(%(c_type)sobj)
{
QapiDeallocVisitor *md;
Visitor *v;
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 45ce3a9..8ffed3d 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -38,7 +38,7 @@ def generate_visit_struct_fields(name, field_prefix,
fn_prefix, members, base =
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;
''',
@@ -149,7 +149,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)
@@ -166,7 +166,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)
{
GenericList *i, **prev = (GenericList **)obj;
Error *err = NULL;
@@ -193,7 +193,7 @@ void visit_type_%(name)sList(Visitor *m, %(name)sList **
obj, const char *name,
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);
}
@@ -203,7 +203,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;
@@ -279,7 +279,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;
@@ -367,13 +367,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)
@@ -383,7 +383,7 @@ def generate_enum_declara
