[Qemu-devel] [PATCH v4 03/14] qapi: Introduce default values for struct members

[Max Reitz]

With this change, it is possible to give default values for struct
members, as follows:

  What you had to do so far:

    # @member: Some description, defaults to 42.
    { 'struct': 'Foo',
      'data': { '*member': 'int' } }

  What you can do now:

    { 'struct': 'Foo',
      'data': { '*member': { 'type': 'int', 'default': 42 } }

On the C side, this change would remove Foo.has_member, because
Foo.member is always valid now.  The input visitor deals with setting
it.  (Naturally, this means that such defaults are useful only for input
parameters.)

At least three things are left unimplemented:

First, support for alternate data types.  This is because supporting
them would mean having to allocate the object in the input visitor, and
then potentially going through multiple levels of nested types.  In any
case, it would have been difficult and I do not think there is need for
such support at this point.

Second, support for null.  The most important reason for this is that
introspection already uses "'default': null" for "no default, but this
field is optional".  The second reason is that without support for
alternate data types, there is not really a point in supporting null.

Third, full support for default lists.  This has a similar reason to the
lack of support for alternate data types: Allocating a default list is
not trivial -- unless the list is empty, which is exactly what we have
support for.

Signed-off-by: Max Reitz <address@hidden>
---
 qapi/introspect.json       |   9 +-
 scripts/qapi/commands.py   |   2 +-
 scripts/qapi/common.py     | 167 +++++++++++++++++++++++++++++++++++--
 scripts/qapi/doc.py        |  20 ++++-
 scripts/qapi/introspect.py |   2 +-
 scripts/qapi/types.py      |   2 +-
 scripts/qapi/visit.py      |  38 ++++++++-
 7 files changed, 217 insertions(+), 23 deletions(-)

diff --git a/qapi/introspect.json b/qapi/introspect.json
index 1843c1cb17..db703135f9 100644
--- a/qapi/introspect.json
+++ b/qapi/introspect.json
@@ -198,11 +198,10 @@
 #
 # @default: default when used as command parameter.
 #           If absent, the parameter is mandatory.
-#           If present, the value must be null.  The parameter is
-#           optional, and behavior when it's missing is not specified
-#           here.
-#           Future extension: if present and non-null, the parameter
-#           is optional, and defaults to this value.
+#           If present and null, the parameter is optional, and
+#           behavior when it's missing is not specified here.
+#           If present and non-null, the parameter is optional, and
+#           defaults to this value.
 #
 # Since: 2.5
 ##
diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
index b929e07be4..6c407cd4ba 100644
--- a/scripts/qapi/commands.py
+++ b/scripts/qapi/commands.py
@@ -35,7 +35,7 @@ def gen_call(name, arg_type, boxed, ret_type):
     elif arg_type:
         assert not arg_type.variants
         for memb in arg_type.members:
-            if memb.optional:
+            if memb.optional and memb.default is None:
                 argstr += 'arg.has_%s, ' % c_name(memb.name)
             argstr += 'arg.%s, ' % c_name(memb.name)
 
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index c6754a5856..8c57d0c67a 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -14,6 +14,7 @@
 from __future__ import print_function
 from contextlib import contextmanager
 import errno
+import math
 import os
 import re
 import string
@@ -800,6 +801,136 @@ def check_if(expr, info):
         check_if_str(ifcond, info)
 
 
+def check_value_str(info, value):
+    return 'g_strdup(%s)' % to_c_string(value) if type(value) is str else False
+
+def check_value_number(info, value):
+    if type(value) is not float:
+        return False
+    if math.isinf(value):
+        return 'INFINITY' if value > 0 else '-INFINITY'
+    elif math.isnan(value):
+        return 'NAN'
+    else:
+        return '%.16e' % value
+
+def check_value_bool(info, value):
+    if type(value) is not bool:
+        return False
+    return 'true' if value else 'false'
+
+def is_int_type(value):
+    if type(value) is int:
+        return True
+    # 'long' does not exist in Python 3
+    try:
+        if type(value) is long:
+            return True
+    except NameError:
+        pass
+
+    return False
+
+def gen_check_value_int(bits):
+    def check_value_int(info, value):
+        if not is_int_type(value) or \
+           value < -(2 ** (bits - 1)) or value >= 2 ** (bits - 1):
+            return False
+        if bits > 32:
+            return '%ill' % value
+        else:
+            return '%i' % value
+
+    return check_value_int
+
+def gen_check_value_uint(bits):
+    def check_value_uint(info, value):
+        if not is_int_type(value) or value < 0 or value >= 2 ** bits:
+            return False
+        if bits > 32:
+            return '%uull' % value
+        elif bits > 16:
+            return '%uu' % value
+        else:
+            return '%u' % value
+
+    return check_value_uint
+
+# Check whether the given value fits the given QAPI type.
+# If so, return a C representation of the value (pointers point to
+# newly allocated objects).
+# Otherwise, raise an exception.
+def check_value(info, qapi_type, value):
+    builtin_type_checks = {
+        'str':      check_value_str,
+        'int':      gen_check_value_int(64),
+        'number':   check_value_number,
+        'bool':     check_value_bool,
+        'int8':     gen_check_value_int(8),
+        'int16':    gen_check_value_int(16),
+        'int32':    gen_check_value_int(32),
+        'int64':    gen_check_value_int(64),
+        'uint8':    gen_check_value_uint(8),
+        'uint16':   gen_check_value_uint(16),
+        'uint32':   gen_check_value_uint(32),
+        'uint64':   gen_check_value_uint(64),
+        'size':     gen_check_value_uint(64),
+    }
+
+    # Cannot support null because that would require a value of "None"
+    # (which is reserved for no default)
+    unsupported_builtin_types = ['null', 'any', 'QType']
+
+    if type(qapi_type) is list:
+        has_list = True
+        qapi_type = qapi_type[0]
+    elif qapi_type.endswith('List'):
+        has_list = True
+        qapi_type = qapi_type[:-4]
+    else:
+        has_list = False
+
+    if has_list:
+        if value == []:
+            return 'NULL'
+        else:
+            raise QAPISemError(info,
+                "Support for non-empty lists as default values has not been " \
+                "implemented yet: '{}'".format(value))
+
+    if qapi_type in builtin_type_checks:
+        c_val = builtin_type_checks[qapi_type](info, value)
+        if not c_val:
+            raise QAPISemError(info,
+                "Value '{}' does not match type {}".format(value, qapi_type))
+        return c_val
+
+    if qapi_type in unsupported_builtin_types:
+        raise QAPISemError(info,
+                           "Cannot specify values for type %s" % qapi_type)
+
+    if qapi_type in enum_types:
+        if not check_value_str(info, value):
+            raise QAPISemError(info,
+                "Enum values must be strings, but '{}' is no string" \
+                        .format(value))
+
+        enum_values = enum_types[qapi_type]['data']
+        for ev in enum_values:
+            if ev['name'] == value:
+                return c_enum_const(qapi_type, value,
+                                    enum_types[qapi_type].get('prefix'))
+
+        raise QAPISemError(info,
+            "Value '{}' does not occur in enum {}".format(value, qapi_type))
+
+    # TODO: Support alternates
+
+    raise QAPISemError(info,
+        "Cannot specify values for type %s (not built-in or an enum)" %
+        qapi_type)
+
+
 def check_type(info, source, value, allow_array=False,
                allow_dict=False, allow_optional=False,
                allow_metas=[]):
@@ -842,15 +973,22 @@ def check_type(info, source, value, allow_array=False,
         if c_name(key, False) == 'u' or c_name(key, False).startswith('has_'):
             raise QAPISemError(info, "Member of %s uses reserved name '%s'"
                                % (source, key))
-        # Todo: allow dictionaries to represent default values of
-        # an optional argument.
+
         check_known_keys(info, "member '%s' of %s" % (key, source),
-                         arg, ['type'], ['if'])
+                         arg, ['type'], ['if', 'default'])
         check_type(info, "Member '%s' of %s" % (key, source),
                    arg['type'], allow_array=True,
                    allow_metas=['built-in', 'union', 'alternate', 'struct',
                                 'enum'])
 
+        if 'default' in arg:
+            if key[0] != '*':
+                raise QAPISemError(info,
+                    "'%s' is not optional, so it cannot have a default value" %
+                    key)
+
+            check_value(info, arg['type'], arg['default'])
+
 
 def check_command(expr, info):
     name = expr['command']
@@ -1601,13 +1739,14 @@ class QAPISchemaFeature(QAPISchemaMember):
 
 
 class QAPISchemaObjectTypeMember(QAPISchemaMember):
-    def __init__(self, name, typ, optional, ifcond=None):
+    def __init__(self, name, typ, optional, ifcond=None, default=None):
         QAPISchemaMember.__init__(self, name, ifcond)
         assert isinstance(typ, str)
         assert isinstance(optional, bool)
         self._type_name = typ
         self.type = None
         self.optional = optional
+        self.default = default
 
     def check(self, schema):
         assert self.owner
@@ -1917,7 +2056,7 @@ class QAPISchema(object):
             name, info, doc, ifcond,
             self._make_enum_members(data), prefix))
 
-    def _make_member(self, name, typ, ifcond, info):
+    def _make_member(self, name, typ, ifcond, default, info):
         optional = False
         if name.startswith('*'):
             name = name[1:]
@@ -1925,10 +2064,11 @@ class QAPISchema(object):
         if isinstance(typ, list):
             assert len(typ) == 1
             typ = self._make_array_type(typ[0], info)
-        return QAPISchemaObjectTypeMember(name, typ, optional, ifcond)
+        return QAPISchemaObjectTypeMember(name, typ, optional, ifcond, default)
 
     def _make_members(self, data, info):
-        return [self._make_member(key, value['type'], value.get('if'), info)
+        return [self._make_member(key, value['type'], value.get('if'),
+                                  value.get('default'), info)
                 for (key, value) in data.items()]
 
     def _def_struct_type(self, expr, info, doc):
@@ -1951,7 +2091,7 @@ class QAPISchema(object):
             typ = self._make_array_type(typ[0], info)
         typ = self._make_implicit_object_type(
             typ, info, None, self.lookup_type(typ),
-            'wrapper', [self._make_member('data', typ, None, info)])
+            'wrapper', [self._make_member('data', typ, None, None, info)])
         return QAPISchemaObjectTypeVariant(case, typ, ifcond)
 
     def _def_union_type(self, expr, info, doc):
@@ -2234,6 +2374,15 @@ def to_c_string(string):
     return result
 
 
+# Translates a value for the given QAPI type to its C representation.
+# The caller must have called check_value() during parsing to be sure
+# that the given value fits the type.
+def c_value(qapi_type, value):
+    pseudo_info = {'file': '(generator bug)', 'line': 0, 'parent': None}
+    # The caller guarantees this does not raise an exception
+    return check_value(pseudo_info, qapi_type, value)
+
+
 def guardstart(name):
     return mcgen('''
 #ifndef %(name)s
@@ -2356,7 +2505,7 @@ def build_params(arg_type, boxed, extra=None):
         for memb in arg_type.members:
             ret += sep
             sep = ', '
-            if memb.optional:
+            if memb.optional and memb.default is None:
                 ret += 'bool has_%s, ' % c_name(memb.name)
             ret += '%s %s' % (memb.type.c_param_type(),
                               c_name(memb.name))
diff --git a/scripts/qapi/doc.py b/scripts/qapi/doc.py
index 5fc0fc7e06..78a9052738 100755
--- a/scripts/qapi/doc.py
+++ b/scripts/qapi/doc.py
@@ -139,13 +139,29 @@ def texi_enum_value(value, desc, suffix):
         value.name, desc, texi_if(value.ifcond, prefix='@*'))
 
 
+def doc_value(value):
+    if value is True:
+        return 'true'
+    elif value is False:
+        return 'false'
+    elif value is None:
+        return 'null'
+    else:
+        return '{}'.format(value)
+
 def texi_member(member, desc, suffix):
     """Format a table of members item for an object type member"""
     typ = member.type.doc_type()
     membertype = ': ' + typ if typ else ''
+
+    optional_info = ''
+    if member.default is not None:
+        optional_info = ' (optional, default: %s)' % doc_value(member.default)
+    elif member.optional:
+        optional_info = ' (optional)'
+
     return '@item @code{%s%s}%s%s\n%s%s' % (
-        member.name, membertype,
-        ' (optional)' if member.optional else '',
+        member.name, membertype, optional_info,
         suffix, desc, texi_if(member.ifcond, prefix='@*'))
 
 
diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
index 572e0b8331..7d73020a42 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -159,7 +159,7 @@ const QLitObject %(c_name)s = %(c_string)s;
     def _gen_member(self, member):
         ret = {'name': member.name, 'type': self._use_type(member.type)}
         if member.optional:
-            ret['default'] = None
+            ret['default'] = member.default
         if member.ifcond:
             ret = (ret, {'if': member.ifcond})
         return ret
diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
index 3edd9374aa..46a6d33379 100644
--- a/scripts/qapi/types.py
+++ b/scripts/qapi/types.py
@@ -44,7 +44,7 @@ def gen_struct_members(members):
     ret = ''
     for memb in members:
         ret += gen_if(memb.ifcond)
-        if memb.optional:
+        if memb.optional and memb.default is None:
             ret += mcgen('''
     bool has_%(c_name)s;
 ''',
diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
index 484ebb66ad..0960e25a25 100644
--- a/scripts/qapi/visit.py
+++ b/scripts/qapi/visit.py
@@ -40,10 +40,14 @@ def gen_visit_object_members(name, base, members, variants):
 void visit_type_%(c_name)s_members(Visitor *v, %(c_name)s *obj, Error **errp)
 {
     Error *err = NULL;
-
 ''',
                 c_name=c_name(name))
 
+    if len([m for m in members if m.default is not None]) > 0:
+        ret += mcgen('''
+    bool has_optional;
+''')
+
     if base:
         ret += mcgen('''
     visit_type_%(c_type)s_members(v, (%(c_type)s *)obj, &err);
@@ -53,13 +57,28 @@ void visit_type_%(c_name)s_members(Visitor *v, %(c_name)s 
*obj, Error **errp)
 ''',
                      c_type=base.c_name())
 
+    ret += mcgen('''
+
+''')
+
     for memb in members:
         ret += gen_if(memb.ifcond)
         if memb.optional:
+            if memb.default is not None:
+                optional_target = 'has_optional'
+                # Visitors other than the input visitor do not have to 
implement
+                # .optional().  Therefore, we have to initialize has_optional.
+                # Initialize it to true, because the field's value is always
+                # present when using any visitor but the input visitor.
+                ret += mcgen('''
+    has_optional = true;
+''')
+            else:
+                optional_target = 'obj->has_' + c_name(memb.name)
             ret += mcgen('''
-    if (visit_optional(v, "%(name)s", &obj->has_%(c_name)s)) {
+    if (visit_optional(v, "%(name)s", &%(opt_target)s)) {
 ''',
-                         name=memb.name, c_name=c_name(memb.name))
+                         name=memb.name, opt_target=optional_target)
             push_indent()
         ret += mcgen('''
     visit_type_%(c_type)s(v, "%(name)s", &obj->%(c_name)s, &err);
@@ -69,7 +88,16 @@ void visit_type_%(c_name)s_members(Visitor *v, %(c_name)s 
*obj, Error **errp)
 ''',
                      c_type=memb.type.c_name(), name=memb.name,
                      c_name=c_name(memb.name))
-        if memb.optional:
+        if memb.default is not None:
+            pop_indent()
+            ret += mcgen('''
+    } else {
+        obj->%(c_name)s = %(c_value)s;
+    }
+''',
+                         c_name=c_name(memb.name),
+                         c_value=c_value(memb._type_name, memb.default))
+        elif memb.optional:
             pop_indent()
             ret += mcgen('''
     }
@@ -287,6 +315,7 @@ class QAPISchemaGenVisitVisitor(QAPISchemaModularCVisitor):
         self._add_system_module(None, ' * Built-in QAPI visitors')
         self._genc.preamble_add(mcgen('''
 #include "qemu/osdep.h"
+#include <math.h>
 #include "qapi/error.h"
 #include "qapi/qapi-builtin-visit.h"
 '''))
@@ -302,6 +331,7 @@ class QAPISchemaGenVisitVisitor(QAPISchemaModularCVisitor):
         visit = self._module_basename('qapi-visit', name)
         self._genc.preamble_add(mcgen('''
 #include "qemu/osdep.h"
+#include <math.h>
 #include "qapi/error.h"
 #include "qapi/qmp/qerror.h"
 #include "%(visit)s.h"
-- 
2.21.0