diff --git a/docs/devel/qapi-code-gen.rst b/docs/devel/qapi-code-gen.rst index 3a817ba498..5edc49aa74 100644 --- a/docs/devel/qapi-code-gen.rst +++ b/docs/devel/qapi-code-gen.rst @@ -1410,7 +1410,6 @@ Example:: struct UserDefOne { int64_t integer; - bool has_string; char *string; bool has_flag; bool flag; @@ -1525,10 +1524,12 @@ Example:: bool visit_type_UserDefOne_members(Visitor *v, UserDefOne *obj, Error **errp) { + bool has_string = !!obj->string; + if (!visit_type_int(v, "integer", &obj->integer, errp)) { return false; } - if (visit_optional(v, "string", &obj->has_string)) { + if (visit_optional(v, "string", &has_string)) { if (!visit_type_str(v, "string", &obj->string, errp)) { return false; } diff --git a/docs/devel/writing-monitor-commands.rst b/docs/devel/writing-monitor-commands.rst index 2fefedcd98..2c11e71665 100644 --- a/docs/devel/writing-monitor-commands.rst +++ b/docs/devel/writing-monitor-commands.rst @@ -166,9 +166,9 @@ and user defined types. Now, let's update our C implementation in monitor/qmp-cmds.c:: - void qmp_hello_world(bool has_message, const char *message, Error **errp) + void qmp_hello_world(const char *message, Error **errp) { - if (has_message) { + if (message) { printf("%s\n", message); } else { printf("Hello, world\n"); @@ -210,9 +210,9 @@ file. Basically, most errors are set by calling the error_setg() function. Let's say we don't accept the string "message" to contain the word "love". If it does contain it, we want the "hello-world" command to return an error:: - void qmp_hello_world(bool has_message, const char *message, Error **errp) + void qmp_hello_world(const char *message, Error **errp) { - if (has_message) { + if (message) { if (strstr(message, "love")) { error_setg(errp, "the word 'love' is not allowed"); return; @@ -467,9 +467,9 @@ There are a number of things to be noticed: allocated by the regular g_malloc0() function. Note that we chose to initialize the memory to zero. This is recommended for all QAPI types, as it helps avoiding bad surprises (specially with booleans) -4. Remember that "next_deadline" is optional? All optional members have a - 'has_TYPE_NAME' member that should be properly set by the implementation, - as shown above +4. Remember that "next_deadline" is optional? Non-pointer optional + members have a 'has_TYPE_NAME' member that should be properly set + by the implementation, as shown above 5. Even static strings, such as "alarm_timer->name", should be dynamically allocated by the implementation. This is so because the QAPI also generates a function to free its types and it cannot distinguish between dynamically diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py index cf68aaf0bf..79c5e5c3a9 100644 --- a/scripts/qapi/commands.py +++ b/scripts/qapi/commands.py @@ -64,7 +64,7 @@ def gen_call(name: str, elif arg_type: assert not arg_type.variants for memb in arg_type.members: - if memb.optional: + if memb.need_has(): argstr += 'arg.has_%s, ' % c_name(memb.name) argstr += 'arg.%s, ' % c_name(memb.name) diff --git a/scripts/qapi/events.py b/scripts/qapi/events.py index e762d53d19..3cf01e96b6 100644 --- a/scripts/qapi/events.py +++ b/scripts/qapi/events.py @@ -60,7 +60,7 @@ def gen_param_var(typ: QAPISchemaObjectType) -> str: for memb in typ.members: ret += sep sep = ', ' - if memb.optional: + if memb.need_has(): ret += 'has_' + c_name(memb.name) + sep if memb.type.name == 'str': # Cast away const added in build_params() diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py index 113b49134d..b5a8d03e8e 100644 --- a/scripts/qapi/gen.py +++ b/scripts/qapi/gen.py @@ -121,7 +121,7 @@ def build_params(arg_type: Optional[QAPISchemaObjectType], for memb in arg_type.members: ret += sep sep = ', ' - if memb.optional: + if memb.need_has(): 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/schema.py b/scripts/qapi/schema.py index 3728340c37..58b00982ea 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -253,6 +253,11 @@ def doc_type(self): return None return self.name + def need_has_if_optional(self): + # When FOO is a pointer, has_FOO == !!FOO, i.e. has_FOO is redundant. + # Except for arrays; see QAPISchemaArrayType.need_has_if_optional(). + return not self.c_type().endswith(POINTER_SUFFIX) + def check(self, schema): QAPISchemaEntity.check(self, schema) for feat in self.features: @@ -352,6 +357,11 @@ def __init__(self, name, info, element_type): self._element_type_name = element_type self.element_type = None + def need_has_if_optional(self): + # When FOO is an array, we still need has_FOO to distinguish + # absent (!has_FOO) from present and empty (has_FOO && !FOO). + return True + def check(self, schema): super().check(schema) self.element_type = schema.resolve_type( @@ -745,6 +755,44 @@ def __init__(self, name, info, typ, optional, ifcond=None, features=None): self.optional = optional self.features = features or [] + def need_has(self): + assert self.type + # Temporary hack to support dropping the has_FOO in reviewable chunks + opt_out = [ + 'qapi/acpi.json', + 'qapi/audio.json', + 'qapi/block-core.json', + 'qapi/block-export.json', + 'qapi/block.json', + 'qapi/char.json', + 'qapi/crypto.json', + 'qapi/dump.json', + 'qapi/introspect.json', + 'qapi/job.json', + 'qapi/machine.json', + 'qapi/machine-target.json', + 'qapi/migration.json', + 'qapi/misc.json', + 'qapi/net.json', + 'qapi/pci.json', + 'qapi/qdev.json', + 'qapi/qom.json', + 'qapi/replay.json', + 'qapi/rocker.json', + 'qapi/run-state.json', + 'qapi/stats.json', + 'qapi/tpm.json', + 'qapi/transaction.json', + 'qapi/ui.json', + 'qapi/virtio.json', + 'qga/qapi-schema.json', + 'tests/qapi-schema/qapi-schema-test.json'] + if self.info and any(self.info.fname.endswith(mod) + for mod in opt_out): + return self.optional + # End of temporary hack + return self.optional and self.type.need_has_if_optional() + def check(self, schema): assert self.defined_in self.type = schema.resolve_type(self._type_name, self.info, diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py index 477d027001..c39d054d2c 100644 --- a/scripts/qapi/types.py +++ b/scripts/qapi/types.py @@ -142,7 +142,7 @@ def gen_struct_members(members: List[QAPISchemaObjectTypeMember]) -> str: ret = '' for memb in members: ret += memb.ifcond.gen_if() - if memb.optional: + if memb.need_has(): ret += mcgen(''' bool has_%(c_name)s; ''', diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py index 380fa197f5..26a584ee4c 100644 --- a/scripts/qapi/visit.py +++ b/scripts/qapi/visit.py @@ -71,6 +71,16 @@ def gen_visit_object_members(name: str, ''', c_name=c_name(name)) + sep = '' + for memb in members: + if memb.optional and not memb.need_has(): + ret += mcgen(''' + bool has_%(c_name)s = !!obj->%(c_name)s; +''', + c_name=c_name(memb.name)) + sep = '\n' + ret += sep + if base: ret += mcgen(''' if (!visit_type_%(c_type)s_members(v, (%(c_type)s *)obj, errp)) { @@ -82,10 +92,13 @@ def gen_visit_object_members(name: str, for memb in members: ret += memb.ifcond.gen_if() if memb.optional: + has = 'has_' + c_name(memb.name) + if memb.need_has(): + has = 'obj->' + has ret += mcgen(''' - if (visit_optional(v, "%(name)s", &obj->has_%(c_name)s)) { + if (visit_optional(v, "%(name)s", &%(has)s)) { ''', - name=memb.name, c_name=c_name(memb.name)) + name=memb.name, has=has) indent.increase() special_features = gen_special_features(memb.features) if special_features != '0':