vppapigen: fix missing vla check for union class

Type: fix
Signed-off-by: Ole Troan <ot@cisco.com>
Change-Id: Ie775cf3469d761847ac39cf0d80a3ec6463b7928
(cherry picked from commit d5a78a5310ff25c0c34ce5773acd8211d48f59f6)
Signed-off-by: Ole Troan <ot@cisco.com>

2 files changed, 111 insertions, 48 deletions

diff --git a/src/tools/vppapigen/test_vppapigen.py b/src/tools/vppapigen/test_vppapigen.py
index a8a0a49a8db..57d4a7a24f8 100755
--- a/src/tools/vppapigen/test_vppapigen.py
+++ b/src/tools/vppapigen/test_vppapigen.py
@@ -1,7 +1,7 @@
 #!/usr/bin/env python3
 
 import unittest
-from vppapigen import VPPAPI, Option, ParseError
+from vppapigen import VPPAPI, Option, ParseError, Union
 
 # TODO
 # - test parsing of options, typedefs, enums, defines, CRC
@@ -18,16 +18,71 @@ class TestVersion(unittest.TestCase):
         r = self.parser.parse_string(version_string)
         self.assertTrue(isinstance(r[0], Option))
 
+class TestUnion(unittest.TestCase):
+    @classmethod
+    def setUpClass(cls):
+        cls.parser = VPPAPI()
+
+    def test_union(self):
+        test_string = '''
+        union foo_union {
+        u32 a;
+        u8 b;
+        };
+        '''
+        r = self.parser.parse_string(test_string)
+        self.assertTrue(isinstance(r[0], Union))
+
+    def test_union_vla(self):
+        test_string = '''
+        union foo_union_vla {
+        u32 a;
+        u8 b[a];
+        };
+        autoreply define foo {
+        vl_api_foo_union_vla_t v;
+        };
+        '''
+        r = self.parser.parse_string(test_string)
+        self.assertTrue(isinstance(r[0], Union))
+        self.assertTrue(r[0].vla)
+        s = self.parser.process(r)
+
+
+        test_string2 = '''
+        union foo_union_vla2 {
+        u32 a;
+        u8 b[a];
+        u32 c;
+        };
+        autoreply define foo2 {
+        vl_api_foo_union_vla2_t v;
+        };
+        '''
+        self.assertRaises(ValueError, self.parser.parse_string, test_string2)
+
+        test_string3 = '''
+        union foo_union_vla3 {
+        u32 a;
+        u8 b[a];
+        };
+        autoreply define foo3 {
+        vl_api_foo_union_vla3_t v;
+        u32 x;
+        };
+        '''
+        self.assertRaises(ValueError, self.parser.parse_string, test_string3)
 
 class TestTypedef(unittest.TestCase):
     @classmethod
     def setUpClass(cls):
         cls.parser = VPPAPI()
 
+    @unittest.skip('Not yet implemented on 19.08')
     def test_duplicatetype(self):
         test_string = '''
-        typeonly define foo1 { u8 dummy; };
-        typeonly define foo1 { u8 dummy; };
+        typedef foo1 { u8 dummy; };
+        typedef foo1 { u8 dummy; };
         '''
         self.assertRaises(KeyError, self.parser.parse_string, test_string)
 
@@ -39,23 +94,29 @@ class TestDefine(unittest.TestCase):
 
     def test_unknowntype(self):
         test_string = 'define foo { foobar foo;};'
-        self.assertRaises(ParseError, self.parser.parse_string, test_string)
+        with self.assertRaises(ParseError) as ctx:
+            self.parser.parse_string(test_string)
+        self.assertIn('Undefined type: foobar', str(ctx.exception))
+
         test_string = 'define { u8 foo;};'
-        self.assertRaises(ParseError, self.parser.parse_string, test_string)
+        with self.assertRaises(ParseError) as ctx:
+            self.parser.parse_string(test_string)
 
     def test_flags(self):
         test_string = '''
           manual_print dont_trace manual_endian define foo { u8 foo; };
+          define foo_reply {u32 context; i32 retval; };
         '''
         r = self.parser.parse_string(test_string)
         self.assertIsNotNone(r)
         s = self.parser.process(r)
         self.assertIsNotNone(s)
-        for d in s['defines']:
-            self.assertTrue(d.dont_trace)
-            self.assertTrue(d.manual_endian)
-            self.assertTrue(d.manual_print)
-            self.assertFalse(d.autoreply)
+        for d in s['Define']:
+            if d.name == 'foo':
+                self.assertTrue(d.dont_trace)
+                self.assertTrue(d.manual_endian)
+                self.assertTrue(d.manual_print)
+                self.assertFalse(d.autoreply)
 
         test_string = '''
           nonexisting_flag define foo { u8 foo; };
@@ -71,11 +132,14 @@ class TestService(unittest.TestCase):
 
     def test_service(self):
         test_string = '''
-         service foo { rpc foo (show_version) returns (show_version) };
+         autoreply define show_version { u8 foo;};
+         service { rpc show_version returns show_version_reply; };
         '''
         r = self.parser.parse_string(test_string)
-        print('R', r)
+        s = self.parser.process(r)
+        self.assertEqual(s['Service'][0].caller, 'show_version')
+        self.assertEqual(s['Service'][0].reply, 'show_version_reply')
 
 
 if __name__ == '__main__':
-    unittest.main()
+    unittest.main(verbosity=2)
diff --git a/src/tools/vppapigen/vppapigen.py b/src/tools/vppapigen/vppapigen.py
index c5304db834c..79f2da91b19 100755
--- a/src/tools/vppapigen/vppapigen.py
+++ b/src/tools/vppapigen/vppapigen.py
@@ -127,6 +127,33 @@ def crc_block_combine(block, crc):
     return binascii.crc32(s, crc) & 0xffffffff
 
 
+def vla_is_last_check(name, block):
+    vla = False
+    for i, b in enumerate(block):
+        if isinstance(b, Array) and b.vla:
+            vla = True
+            if i + 1 < len(block):
+                raise ValueError(
+                    'VLA field "{}" must be the last field in message "{}"'
+                    .format(b.fieldname, name))
+        elif b.fieldtype.startswith('vl_api_'):
+            if global_types[b.fieldtype].vla:
+                vla = True
+                if i + 1 < len(block):
+                    raise ValueError(
+                        'VLA field "{}" must be the last '
+                        'field in message "{}"'
+                        .format(b.fieldname, name))
+        elif b.fieldtype == 'string' and b.length == 0:
+            vla = True
+            if i + 1 < len(block):
+                raise ValueError(
+                    'VLA field "{}" must be the last '
+                    'field in message "{}"'
+                    .format(b.fieldname, name))
+    return vla
+
+
 class Service():
     def __init__(self, caller, reply, events=None, stream=False):
         self.caller = caller
@@ -148,26 +175,10 @@ class Typedef():
                 self.manual_print = True
             elif f == 'manual_endian':
                 self.manual_endian = True
-        global_type_add(name, self)
 
-        self.vla = False
+        global_type_add(name, self)
 
-        for i, b in enumerate(block):
-            if isinstance(b, Array):
-                if b.length == 0:
-                    self.vla = True
-                    if i + 1 < len(block):
-                        raise ValueError(
-                            'VLA field "{}" must be the last '
-                            'field in message "{}"'
-                            .format(b.fieldname, name))
-            elif b.fieldtype == 'string':
-                self.vla = True
-                if i + 1 < len(block):
-                    raise ValueError(
-                        'VLA field "{}" must be the last '
-                        'field in message "{}"'
-                        .format(b.fieldname, name))
+        self.vla = vla_is_last_check(name, block)
 
     def __repr__(self):
         return self.name + str(self.flags) + str(self.block)
@@ -200,8 +211,11 @@ class Union():
         self.manual_print = False
         self.manual_endian = False
         self.name = name
+
         self.block = block
         self.crc = str(block).encode()
+        self.vla = vla_is_last_check(name, block)
+
         global_type_add(name, self)
 
     def __repr__(self):
@@ -229,28 +243,13 @@ class Define():
             elif f == 'autoreply':
                 self.autoreply = True
 
-        for i, b in enumerate(block):
+        for b in block:
             if isinstance(b, Option):
                 if b[1] == 'singular' and b[2] == 'true':
                     self.singular = True
                 block.remove(b)
 
-            if isinstance(b, Array) and b.vla and i + 1 < len(block):
-                raise ValueError(
-                    'VLA field "{}" must be the last field in message "{}"'
-                    .format(b.fieldname, name))
-            elif b.fieldtype.startswith('vl_api_'):
-                if (global_types[b.fieldtype].vla and i + 1 < len(block)):
-                    raise ValueError(
-                        'VLA field "{}" must be the last '
-                        'field in message "{}"'
-                        .format(b.fieldname, name))
-            elif b.fieldtype == 'string' and b.length == 0:
-                if i + 1 < len(block):
-                    raise ValueError(
-                        'VLA field "{}" must be the last '
-                        'field in message "{}"'
-                        .format(b.fieldname, name))
+        self.vla = vla_is_last_check(name, block)
 
     def __repr__(self):
         return self.name + str(self.flags) + str(self.block)