diff mbox

[V2,5/8] qapi script: use same function to generate enum string

Message ID 1384295108-10913-6-git-send-email-xiawenc@linux.vnet.ibm.com
State New
Headers show

Commit Message

Wayne Xia Nov. 12, 2013, 10:25 p.m. UTC
One function one rule, so the enum string generating have same
behavior for different caller. If multiple caller exist for one
enum define in schema, it is for sure the generated string is
identical.

Note before the patch qapi-visit.py used custom function to
generate the string in union visit, although the patch changes it,
the final string generated is not changed. The custom function used
before will met problem when capitalized discriminator value is
introduced.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 scripts/qapi-types.py |    6 +++---
 scripts/qapi-visit.py |   21 +++++++++++++++------
 scripts/qapi.py       |   15 +++++++++++----
 3 files changed, 29 insertions(+), 13 deletions(-)

Comments

Eric Blake Dec. 2, 2013, 8:17 p.m. UTC | #1
On 11/12/2013 03:25 PM, Wenchao Xia wrote:
> One function one rule, so the enum string generating have same
> behavior for different caller. If multiple caller exist for one
> enum define in schema, it is for sure the generated string is
> identical.
> 
> Note before the patch qapi-visit.py used custom function to
> generate the string in union visit, although the patch changes it,
> the final string generated is not changed. The custom function used
> before will met problem when capitalized discriminator value is
> introduced.

I'm finding this commit message awkward to read and rather wordy.  May I
suggest the shorter:

Prior to this patch, qapi-visit.py used custom code to generate enum
names used for handling a qapi union.  Fix it to instead reuse common
code, with identical generated results, and allowing future updates to
generation to only need to touch one place.
Wayne Xia Dec. 3, 2013, 2:55 a.m. UTC | #2
于 2013/12/3 4:17, Eric Blake 写道:
> On 11/12/2013 03:25 PM, Wenchao Xia wrote:
>> One function one rule, so the enum string generating have same
>> behavior for different caller. If multiple caller exist for one
>> enum define in schema, it is for sure the generated string is
>> identical.
>>
>> Note before the patch qapi-visit.py used custom function to
>> generate the string in union visit, although the patch changes it,
>> the final string generated is not changed. The custom function used
>> before will met problem when capitalized discriminator value is
>> introduced.
>
> I'm finding this commit message awkward to read and rather wordy.  May I
> suggest the shorter:
>
> Prior to this patch, qapi-visit.py used custom code to generate enum
> names used for handling a qapi union.  Fix it to instead reuse common
> code, with identical generated results, and allowing future updates to
> generation to only need to touch one place.
>
   That sounds better, thanks for reviewing. I have sent v3 before at:
http://lists.nongnu.org/archive/html/qemu-devel/2013-11/msg03980.html
There is no change except better intro/doc, will use the new commit
message in V4.
diff mbox

Patch

diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 88bf76a..914f055 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -144,11 +144,11 @@  typedef enum %(name)s
 
     i = 0
     for value in enum_values:
+        enum_full_value_string = generate_enum_full_value_string(name, value)
         enum_decl += mcgen('''
-    %(abbrev)s_%(value)s = %(i)d,
+    %(enum_full_value_string)s = %(i)d,
 ''',
-                     abbrev=de_camel_case(name).upper(),
-                     value=generate_enum_name(value),
+                     enum_full_value_string=enum_full_value_string,
                      i=i)
         i += 1
 
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 61e03a1..1769470 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -208,18 +208,23 @@  void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **
 ''',
     name=name)
 
+    # For anon union, always use the default enum type automatically generated
+    # as "'%sKind' % (name)"
+    discriminator_type_name = '%sKind' % (name)
+
     for key in members:
         assert (members[key] in builtin_types
             or find_struct(members[key])
             or find_union(members[key])), "Invalid anonymous union member"
 
+        enum_full_value_string = \
+                  generate_enum_full_value_string(discriminator_type_name, key)
         ret += mcgen('''
-        case %(abbrev)s_KIND_%(enum)s:
+        case %(enum_full_value_string)s:
             visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, name, &err);
             break;
 ''',
-                abbrev = de_camel_case(name).upper(),
-                enum = c_fun(de_camel_case(key),False).upper(),
+                enum_full_value_string = enum_full_value_string,
                 c_type = type_name(members[key]),
                 c_name = c_fun(key))
 
@@ -266,7 +271,10 @@  def generate_visit_union(expr):
                                  (key, name))
                 sys.exit(1)
 
+    # There will always be a discriminator in the C switch code, by default it
+    # is an enum type generated silently as "'%sKind' % (name)"
     ret = generate_visit_enum('%sKind' % name, members.keys())
+    discriminator_type_name = '%sKind' % (name)
 
     if base:
         base_fields = find_struct(base)['data']
@@ -319,13 +327,14 @@  void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **
                     visit_end_implicit_struct(m, &err);
                 }'''
 
+        enum_full_value_string = \
+                  generate_enum_full_value_string(discriminator_type_name, key)
         ret += mcgen('''
-            case %(abbrev)s_KIND_%(enum)s:
+            case %(enum_full_value_string)s:
                 ''' + fmt + '''
                 break;
 ''',
-                abbrev = de_camel_case(name).upper(),
-                enum = c_fun(de_camel_case(key),False).upper(),
+                enum_full_value_string = enum_full_value_string,
                 c_type=type_name(members[key]),
                 c_name=c_fun(key))
 
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 0e4e9d7..28ede6f 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -417,12 +417,19 @@  def discriminator_find_enum_define(expr):
 
     return find_enum(discriminator_type)
 
-def generate_enum_name(name):
-    if name.isupper():
-        return c_fun(name, False)
+def _generate_enum_value_string(value):
+    if value.isupper():
+        return c_fun(value, False)
     new_name = ''
-    for c in c_fun(name, False):
+    for c in c_fun(value, False):
         if c.isupper():
             new_name += '_'
         new_name += c
     return new_name.lstrip('_').upper()
+
+def generate_enum_full_value_string(enum_name, enum_value):
+    # generate abbrev string
+    abbrev_string = de_camel_case(enum_name).upper()
+    # generate value string
+    value_string = _generate_enum_value_string(enum_value)
+    return "%s_%s" % (abbrev_string, value_string)