Re: [PATCH v3 2/3] qapi: Do not generate empty enum

[Philippe Mathieu-Daudé]

On 16/3/23 15:57, Markus Armbruster wrote:

Daniel P. Berrangé <berra...@redhat.com> writes:

But all of this falls apart with conditional members!

Example 1 (taken from qapi/block-core.json):

     { 'enum': 'BlockdevAioOptions',
       'data': [ 'threads', 'native',
                 { 'name': 'io_uring', 'if': 'CONFIG_LINUX_IO_URING' } ] }

Generates now:

     typedef enum BlockdevAioOptions {
         BLOCKDEV_AIO_OPTIONS_THREADS,
         BLOCKDEV_AIO_OPTIONS_NATIVE,
     #if defined(CONFIG_LINUX_IO_URING)
         BLOCKDEV_AIO_OPTIONS_IO_URING,
     #endif /* defined(CONFIG_LINUX_IO_URING) */
         BLOCKDEV_AIO_OPTIONS__MAX,
     } BlockdevAioOptions;

BLOCKDEV_AIO_OPTIONS__MAX is 3 if defined(CONFIG_LINUX_IO_URING), else
2.

After the next patch:

     typedef enum BlockdevAioOptions {
         BLOCKDEV_AIO_OPTIONS_THREADS,
         BLOCKDEV_AIO_OPTIONS_NATIVE,
     #if defined(CONFIG_LINUX_IO_URING)
         BLOCKDEV_AIO_OPTIONS_IO_URING,
     #endif /* defined(CONFIG_LINUX_IO_URING) */
     #define BLOCKDEV_AIO_OPTIONS__MAX 3
     } BlockdevAioOptions;

Now it's always 3.

Example 2 (same with members reordered):

     { 'enum': 'BlockdevAioOptions',
       'data': [ { 'name': 'io_uring', 'if': 'CONFIG_LINUX_IO_URING' },
                 'threads', 'native' ] }

Same problem for __MAX, additional problem for __DUMMY:

     typedef enum BlockdevAioOptions {
         BLOCKDEV_AIO_OPTIONS__DUMMY = 0,
     #if defined(CONFIG_LINUX_IO_URING)
         BLOCKDEV_AIO_OPTIONS_IO_URING = 0,
     #endif /* defined(CONFIG_LINUX_IO_URING) */
         BLOCKDEV_AIO_OPTIONS_THREADS,
         BLOCKDEV_AIO_OPTIONS_NATIVE,
     #define BLOCKDEV_AIO_OPTIONS__MAX 3
     } BlockdevAioOptions;

If CONFIG_LINUX_IO_URING is off, the enum starts at 1 instead of 0.

Arrays indexed by the enum start with a hole.  Code using them is
probably not prepared for holes.

Can we meet half-way only generating the MAX definitions for
unconditional enums, keeping the conditional ones as is?

-- >8 --
diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
@@ -88,16 +88,18 @@ def gen_enum(name: str,
              members: List[QAPISchemaEnumMember],
              prefix: Optional[str] = None) -> str:
     assert members
-    # append automatically generated _MAX value
-    enum_members = members + [QAPISchemaEnumMember('_MAX', None)]
-
     ret = mcgen('''

 typedef enum %(c_name)s {
 ''',
                 c_name=c_name(name))

-    for memb in enum_members:
+    has_cond = any(memb.ifcond.is_present() for memb in members)
+    if has_cond:
+        # append automatically generated _MAX value
+        members += [QAPISchemaEnumMember('_MAX', None)]
+
+    for memb in members:
         ret += memb.ifcond.gen_if()
         ret += mcgen('''
     %(c_enum)s,
@@ -105,6 +107,13 @@ def gen_enum(name: str,
                      c_enum=c_enum_const(name, memb.name, prefix))
         ret += memb.ifcond.gen_endif()

+    if not has_cond:
+        ret += mcgen('''
+#define %(c_name)s %(c_length)s
+''',
+                     c_name=c_enum_const(name, '_MAX', prefix),
+                     c_length=len(members))
+
     ret += mcgen('''
 } %(c_name)s;
 ''',
---