qapi: Replace start_optional()/end_optional() by optional() Semantics of end_optional() differ subtly from the other end_FOO() callbacks: when start_FOO() succeeds, the matching end_FOO() gets called regardless of what happens in between. end_optional() gets called only when everything in between succeeds as well. Entirely undocumented, like all of the visitor API. The only user of Visitor Callback end_optional() never did anything, and was removed in commit 9f9ab46. I'm about to clean up error handling in the generated visitor code, and end_optional() is in my way. No users mean no test cases, and making non-trivial cleanup transformations without test cases doesn't strike me as a good idea. Drop end_optional(), and rename start_optional() to optional(). We can always go back to a pair of callbacks when we have an actual need. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h index 166aadd..ecc0183 100644 --- a/include/qapi/visitor-impl.h +++ b/include/qapi/visitor-impl.h
@@ -42,9 +42,8 @@ Error **errp); /* May be NULL */ - void (*start_optional)(Visitor *v, bool *present, const char *name, - Error **errp); - void (*end_optional)(Visitor *v, Error **errp); + void (*optional)(Visitor *v, bool *present, const char *name, + Error **errp); void (*type_uint8)(Visitor *v, uint8_t *obj, const char *name, Error **errp); void (*type_uint16)(Visitor *v, uint16_t *obj, const char *name, Error **errp);
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h index 29da211..4a0178f 100644 --- a/include/qapi/visitor.h +++ b/include/qapi/visitor.h
@@ -39,9 +39,8 @@ void visit_start_list(Visitor *v, const char *name, Error **errp); GenericList *visit_next_list(Visitor *v, GenericList **list, Error **errp); void visit_end_list(Visitor *v, Error **errp); -void visit_start_optional(Visitor *v, bool *present, const char *name, - Error **errp); -void visit_end_optional(Visitor *v, Error **errp); +void visit_optional(Visitor *v, bool *present, const char *name, + Error **errp); void visit_get_next_type(Visitor *v, int *obj, const int *qtypes, const char *name, Error **errp); void visit_type_enum(Visitor *v, int *obj, const char *strings[],
diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c index 87c1c78..16382e7 100644 --- a/qapi/opts-visitor.c +++ b/qapi/opts-visitor.c
@@ -484,8 +484,7 @@ static void -opts_start_optional(Visitor *v, bool *present, const char *name, - Error **errp) +opts_optional(Visitor *v, bool *present, const char *name, Error **errp) { OptsVisitor *ov = DO_UPCAST(OptsVisitor, visitor, v); @@ -528,7 +527,7 @@ /* type_number() is not filled in, but this is not the first visitor to * skip some mandatory methods... */ - ov->visitor.start_optional = &opts_start_optional; + ov->visitor.optional = &opts_optional; ov->opts_root = opts;
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c index 1f7475c..ffd7637 100644 --- a/qapi/qapi-visit-core.c +++ b/qapi/qapi-visit-core.c
@@ -69,18 +69,11 @@ v->end_list(v, errp); } -void visit_start_optional(Visitor *v, bool *present, const char *name, - Error **errp) +void visit_optional(Visitor *v, bool *present, const char *name, + Error **errp) { - if (!error_is_set(errp) && v->start_optional) { - v->start_optional(v, present, name, errp); - } -} - -void visit_end_optional(Visitor *v, Error **errp) -{ - if (!error_is_set(errp) && v->end_optional) { - v->end_optional(v, errp); + if (!error_is_set(errp) && v->optional) { + v->optional(v, present, name, errp); } }
diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c index a2bed1e..d861206 100644 --- a/qapi/qmp-input-visitor.c +++ b/qapi/qmp-input-visitor.c
@@ -286,8 +286,8 @@ } } -static void qmp_input_start_optional(Visitor *v, bool *present, - const char *name, Error **errp) +static void qmp_input_optional(Visitor *v, bool *present, const char *name, + Error **errp) { QmpInputVisitor *qiv = to_qiv(v); QObject *qobj = qmp_input_get_object(qiv, name, true); @@ -329,7 +329,7 @@ v->visitor.type_bool = qmp_input_type_bool; v->visitor.type_str = qmp_input_type_str; v->visitor.type_number = qmp_input_type_number; - v->visitor.start_optional = qmp_input_start_optional; + v->visitor.optional = qmp_input_optional; v->visitor.get_next_type = qmp_input_get_next_type; qmp_input_push(v, obj, NULL);
diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c index 793548a..5780944 100644 --- a/qapi/string-input-visitor.c +++ b/qapi/string-input-visitor.c
@@ -120,8 +120,8 @@ *obj = val; } -static void parse_start_optional(Visitor *v, bool *present, - const char *name, Error **errp) +static void parse_optional(Visitor *v, bool *present, const char *name, + Error **errp) { StringInputVisitor *siv = DO_UPCAST(StringInputVisitor, visitor, v); @@ -155,7 +155,7 @@ v->visitor.type_bool = parse_type_bool; v->visitor.type_str = parse_type_str; v->visitor.type_number = parse_type_number; - v->visitor.start_optional = parse_start_optional; + v->visitor.optional = parse_optional; v->string = str; return v;
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py index 1399826..341dba2 100644 --- a/scripts/qapi-commands.py +++ b/scripts/qapi-commands.py
@@ -131,7 +131,7 @@ for argname, argtype, optional, structured in parse_args(args): if optional: ret += mcgen(''' -visit_start_optional(v, &has_%(c_name)s, "%(name)s", %(errp)s); +visit_optional(v, &has_%(c_name)s, "%(name)s", %(errp)s); if (has_%(c_name)s) { ''', c_name=c_var(argname), name=argname, errp=errparg) @@ -145,8 +145,7 @@ pop_indent() ret += mcgen(''' } -visit_end_optional(v, %(errp)s); -''', errp=errparg) +''') if dealloc: ret += mcgen('''
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index c6579be..9f22eee 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py
@@ -61,7 +61,7 @@ for argname, argentry, optional, structured in parse_args(members): if optional: ret += mcgen(''' -visit_start_optional(m, &(*obj)->%(c_prefix)shas_%(c_name)s, "%(name)s", &err); +visit_optional(m, &(*obj)->%(c_prefix)shas_%(c_name)s, "%(name)s", &err); if ((*obj)->%(prefix)shas_%(c_name)s) { ''', c_prefix=c_var(field_prefix), prefix=field_prefix, @@ -82,7 +82,6 @@ pop_indent() ret += mcgen(''' } -visit_end_optional(m, &err); ''') pop_indent()