| Message ID | 1460131992-32278-3-git-send-email-eblake@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Eric Blake <eblake@redhat.com> writes: > We have three classes of QAPI visitors: input, output, and dealloc. > Currently, all implementations of these visitors have one thing in > common based on their visitor type: the implementation used for the > visit_type_enum() callback. But since we plan to add more such > common behavior, in relation to documenting and further refining > the semantics, it makes more sense to have the visitor > implementations advertise which class they belong to, so the common > qapi-visit-core code can use that information in multiple places. > > For this patch, knowing the class of a visitor implementation lets > us make input_type_enum() and output_type_enum() become static > functions, by replacing the callback function Visitor.type_enum() > with the simpler enum member Visitor.type. Share a common > assertion in qapi-visit-core as part of the refactoring. > > Signed-off-by: Eric Blake <eblake@redhat.com> > > --- > v14: no change > v13: no change > v12: new patch > --- > include/qapi/visitor-impl.h | 21 ++++++++++++--------- > qapi/qapi-visit-core.c | 28 +++++++++++++++------------- > qapi/opts-visitor.c | 12 ++---------- > qapi/qapi-dealloc-visitor.c | 7 +------ > qapi/qmp-input-visitor.c | 2 +- > qapi/qmp-output-visitor.c | 2 +- > qapi/string-input-visitor.c | 2 +- > qapi/string-output-visitor.c | 2 +- > 8 files changed, 34 insertions(+), 42 deletions(-) > > diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h > index 2bd8f29..228a2a6 100644 > --- a/include/qapi/visitor-impl.h > +++ b/include/qapi/visitor-impl.h > @@ -14,6 +14,15 @@ > > #include "qapi/visitor.h" > > +/* There are three classes of visitors; setting the class determines > + * how QAPI enums are visited, as well as what additional restrictions > + * can be asserted. */ > +typedef enum VisitorType { > + VISITOR_INPUT, > + VISITOR_OUTPUT, > + VISITOR_DEALLOC, > +} VisitorType; > + > struct Visitor > { > /* Must be set */ I think we should explain what makes a visitor an input/output/dealloc visitor. Not necessarily in this patch, and not necessarily in this place, just somewhere. Right now, the information is scattered. > @@ -36,10 +45,6 @@ struct Visitor > void (*end_alternate)(Visitor *v); > > /* Must be set. */ > - void (*type_enum)(Visitor *v, const char *name, int *obj, > - const char *const strings[], Error **errp); > - > - /* Must be set. */ > void (*type_int64)(Visitor *v, const char *name, int64_t *obj, > Error **errp); > /* Must be set. */ > @@ -58,11 +63,9 @@ struct Visitor > > /* May be NULL; most useful for input visitors. */ > void (*optional)(Visitor *v, const char *name, bool *present); > + > + /* Must be set. */ > + VisitorType type; > }; > > -void input_type_enum(Visitor *v, const char *name, int *obj, > - const char *const strings[], Error **errp); > -void output_type_enum(Visitor *v, const char *name, int *obj, > - const char *const strings[], Error **errp); > - > #endif > diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c > index fa680c9..3cd7edc 100644 > --- a/qapi/qapi-visit-core.c > +++ b/qapi/qapi-visit-core.c > @@ -72,12 +72,6 @@ bool visit_optional(Visitor *v, const char *name, bool *present) > return *present; > } > > -void visit_type_enum(Visitor *v, const char *name, int *obj, > - const char *const strings[], Error **errp) > -{ > - v->type_enum(v, name, obj, strings, errp); > -} > - > void visit_type_int(Visitor *v, const char *name, int64_t *obj, Error **errp) > { > v->type_int64(v, name, obj, errp); > @@ -208,14 +202,13 @@ void visit_type_any(Visitor *v, const char *name, QObject **obj, Error **errp) > v->type_any(v, name, obj, errp); > } > > -void output_type_enum(Visitor *v, const char *name, int *obj, > - const char *const strings[], Error **errp) > +static void output_type_enum(Visitor *v, const char *name, int *obj, > + const char *const strings[], Error **errp) > { > int i = 0; > int value = *obj; > char *enum_str; > > - assert(strings); > while (strings[i++] != NULL); > if (value < 0 || value >= i - 1) { > error_setg(errp, QERR_INVALID_PARAMETER, name ? name : "null"); > @@ -226,15 +219,13 @@ void output_type_enum(Visitor *v, const char *name, int *obj, > visit_type_str(v, name, &enum_str, errp); > } > > -void input_type_enum(Visitor *v, const char *name, int *obj, > - const char *const strings[], Error **errp) > +static void input_type_enum(Visitor *v, const char *name, int *obj, > + const char *const strings[], Error **errp) > { > Error *local_err = NULL; > int64_t value = 0; > char *enum_str; > > - assert(strings); > - > visit_type_str(v, name, &enum_str, &local_err); > if (local_err) { > error_propagate(errp, local_err); > @@ -257,3 +248,14 @@ void input_type_enum(Visitor *v, const char *name, int *obj, > g_free(enum_str); > *obj = value; > } > + > +void visit_type_enum(Visitor *v, const char *name, int *obj, > + const char *const strings[], Error **errp) > +{ > + assert(strings); > + if (v->type == VISITOR_INPUT) { > + input_type_enum(v, name, obj, strings, errp); > + } else if (v->type == VISITOR_OUTPUT) { > + output_type_enum(v, name, obj, strings, errp); > + } > +} > diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c > index 602f260..f98cf2e 100644 > --- a/qapi/opts-visitor.c > +++ b/qapi/opts-visitor.c > @@ -507,6 +507,8 @@ opts_visitor_new(const QemuOpts *opts) > > ov = g_malloc0(sizeof *ov); > > + ov->visitor.type = VISITOR_INPUT; > + > ov->visitor.start_struct = &opts_start_struct; > ov->visitor.end_struct = &opts_end_struct; > > @@ -514,16 +516,6 @@ opts_visitor_new(const QemuOpts *opts) > ov->visitor.next_list = &opts_next_list; > ov->visitor.end_list = &opts_end_list; > > - /* input_type_enum() covers both "normal" enums and union discriminators. > - * The union discriminator field is always generated as "type"; it should > - * match the "type" QemuOpt child of any QemuOpts. > - * > - * input_type_enum() will remove the looked-up key from the > - * "unprocessed_opts" hash even if the lookup fails, because the removal is > - * done earlier in opts_type_str(). This should be harmless. > - */ > - ov->visitor.type_enum = &input_type_enum; > - Hmm, this comment doesn't look worthless. With its statement gone, I guess it should move somewhere else. What do you think? > ov->visitor.type_int64 = &opts_type_int64; > ov->visitor.type_uint64 = &opts_type_uint64; > ov->visitor.type_size = &opts_type_size; > diff --git a/qapi/qapi-dealloc-visitor.c b/qapi/qapi-dealloc-visitor.c > index 6922179..c19a459 100644 > --- a/qapi/qapi-dealloc-visitor.c > +++ b/qapi/qapi-dealloc-visitor.c > @@ -163,11 +163,6 @@ static void qapi_dealloc_type_anything(Visitor *v, const char *name, > } > } > > -static void qapi_dealloc_type_enum(Visitor *v, const char *name, int *obj, > - const char * const strings[], Error **errp) > -{ > -} > - > Visitor *qapi_dealloc_get_visitor(QapiDeallocVisitor *v) > { > return &v->visitor; > @@ -184,6 +179,7 @@ QapiDeallocVisitor *qapi_dealloc_visitor_new(void) > > v = g_malloc0(sizeof(*v)); > > + v->visitor.type = VISITOR_DEALLOC; > v->visitor.start_struct = qapi_dealloc_start_struct; > v->visitor.end_struct = qapi_dealloc_end_struct; > v->visitor.start_alternate = qapi_dealloc_start_alternate; > @@ -191,7 +187,6 @@ QapiDeallocVisitor *qapi_dealloc_visitor_new(void) > v->visitor.start_list = qapi_dealloc_start_list; > v->visitor.next_list = qapi_dealloc_next_list; > v->visitor.end_list = qapi_dealloc_end_list; > - v->visitor.type_enum = qapi_dealloc_type_enum; > v->visitor.type_int64 = qapi_dealloc_type_int64; > v->visitor.type_uint64 = qapi_dealloc_type_uint64; > v->visitor.type_bool = qapi_dealloc_type_bool; > diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c > index 7cd1b77..02d4233 100644 > --- a/qapi/qmp-input-visitor.c > +++ b/qapi/qmp-input-visitor.c > @@ -339,13 +339,13 @@ QmpInputVisitor *qmp_input_visitor_new(QObject *obj) > > v = g_malloc0(sizeof(*v)); > > + v->visitor.type = VISITOR_INPUT; > v->visitor.start_struct = qmp_input_start_struct; > v->visitor.end_struct = qmp_input_end_struct; > v->visitor.start_list = qmp_input_start_list; > v->visitor.next_list = qmp_input_next_list; > v->visitor.end_list = qmp_input_end_list; > v->visitor.start_alternate = qmp_input_start_alternate; > - v->visitor.type_enum = input_type_enum; > v->visitor.type_int64 = qmp_input_type_int64; > v->visitor.type_uint64 = qmp_input_type_uint64; > v->visitor.type_bool = qmp_input_type_bool; > diff --git a/qapi/qmp-output-visitor.c b/qapi/qmp-output-visitor.c > index d44c676..1f2a7ba 100644 > --- a/qapi/qmp-output-visitor.c > +++ b/qapi/qmp-output-visitor.c > @@ -234,12 +234,12 @@ QmpOutputVisitor *qmp_output_visitor_new(void) > > v = g_malloc0(sizeof(*v)); > > + v->visitor.type = VISITOR_OUTPUT; > v->visitor.start_struct = qmp_output_start_struct; > v->visitor.end_struct = qmp_output_end_struct; > v->visitor.start_list = qmp_output_start_list; > v->visitor.next_list = qmp_output_next_list; > v->visitor.end_list = qmp_output_end_list; > - v->visitor.type_enum = output_type_enum; > v->visitor.type_int64 = qmp_output_type_int64; > v->visitor.type_uint64 = qmp_output_type_uint64; > v->visitor.type_bool = qmp_output_type_bool; > diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c > index ab12953..d604575 100644 > --- a/qapi/string-input-visitor.c > +++ b/qapi/string-input-visitor.c > @@ -348,7 +348,7 @@ StringInputVisitor *string_input_visitor_new(const char *str) > > v = g_malloc0(sizeof(*v)); > > - v->visitor.type_enum = input_type_enum; > + v->visitor.type = VISITOR_INPUT; > v->visitor.type_int64 = parse_type_int64; > v->visitor.type_uint64 = parse_type_uint64; > v->visitor.type_size = parse_type_size; > diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c > index c2e5c5b..0d44d7e 100644 > --- a/qapi/string-output-visitor.c > +++ b/qapi/string-output-visitor.c > @@ -351,7 +351,7 @@ StringOutputVisitor *string_output_visitor_new(bool human) > > v->string = g_string_new(NULL); > v->human = human; > - v->visitor.type_enum = output_type_enum; > + v->visitor.type = VISITOR_OUTPUT; > v->visitor.type_int64 = print_type_int64; > v->visitor.type_uint64 = print_type_uint64; > v->visitor.type_size = print_type_size;
On 04/13/2016 07:49 AM, Markus Armbruster wrote: > Eric Blake <eblake@redhat.com> writes: > >> We have three classes of QAPI visitors: input, output, and dealloc. >> Currently, all implementations of these visitors have one thing in >> common based on their visitor type: the implementation used for the >> visit_type_enum() callback. But since we plan to add more such >> common behavior, in relation to documenting and further refining >> the semantics, it makes more sense to have the visitor >> implementations advertise which class they belong to, so the common >> qapi-visit-core code can use that information in multiple places. >> >> For this patch, knowing the class of a visitor implementation lets >> us make input_type_enum() and output_type_enum() become static >> functions, by replacing the callback function Visitor.type_enum() >> with the simpler enum member Visitor.type. Share a common >> assertion in qapi-visit-core as part of the refactoring. >> >> Signed-off-by: Eric Blake <eblake@redhat.com> >> >> +/* There are three classes of visitors; setting the class determines >> + * how QAPI enums are visited, as well as what additional restrictions >> + * can be asserted. */ >> +typedef enum VisitorType { >> + VISITOR_INPUT, >> + VISITOR_OUTPUT, >> + VISITOR_DEALLOC, >> +} VisitorType; >> + >> struct Visitor >> { >> /* Must be set */ > > I think we should explain what makes a visitor an input/output/dealloc > visitor. Not necessarily in this patch, and not necessarily in this > place, just somewhere. Right now, the information is scattered. 8/19 might be the patch that does just that. We'll see what you think when you get further through the review. >> @@ -514,16 +516,6 @@ opts_visitor_new(const QemuOpts *opts) >> ov->visitor.next_list = &opts_next_list; >> ov->visitor.end_list = &opts_end_list; >> >> - /* input_type_enum() covers both "normal" enums and union discriminators. >> - * The union discriminator field is always generated as "type"; it should >> - * match the "type" QemuOpt child of any QemuOpts. >> - * >> - * input_type_enum() will remove the looked-up key from the >> - * "unprocessed_opts" hash even if the lookup fails, because the removal is >> - * done earlier in opts_type_str(). This should be harmless. >> - */ >> - ov->visitor.type_enum = &input_type_enum; >> - > > Hmm, this comment doesn't look worthless. With its statement gone, I > guess it should move somewhere else. What do you think? The first half of the comment is fluff. The second half, about a looked-up key being removed from unprocessed_opts even if lookup fails, might be something I can move, but where? Maybe to the visit_type_enum() in qapi-visit-core.c, stating that an input visitor will visit the string even if conversion to enum fails? It really only affects what happens for an input visitor that has a visit_check_struct() (commit 14/19 of the series), but even then, we really only report an input visit failure regarding unvisited options if there was no earlier error - but the mere fact that visiting an enum type fails whether the string was present but not a valid enum value, or whether the string was not even present, means that we won't be reaching the visit_check_struct() to even care about errors about unvisited members. Maybe that means I just move the documentation into the commit message, and explain why the comment disappears (because a later patch will guarantee the semantics that we only care about reporting unvisited members in an input visitor only if all other visits are successful, so it doesn't matter on earlier failure whether we consumed or did not consume input).
Eric Blake <eblake@redhat.com> writes: > On 04/13/2016 07:49 AM, Markus Armbruster wrote: >> Eric Blake <eblake@redhat.com> writes: >> >>> We have three classes of QAPI visitors: input, output, and dealloc. >>> Currently, all implementations of these visitors have one thing in >>> common based on their visitor type: the implementation used for the >>> visit_type_enum() callback. But since we plan to add more such >>> common behavior, in relation to documenting and further refining >>> the semantics, it makes more sense to have the visitor >>> implementations advertise which class they belong to, so the common >>> qapi-visit-core code can use that information in multiple places. >>> >>> For this patch, knowing the class of a visitor implementation lets >>> us make input_type_enum() and output_type_enum() become static >>> functions, by replacing the callback function Visitor.type_enum() >>> with the simpler enum member Visitor.type. Share a common >>> assertion in qapi-visit-core as part of the refactoring. >>> >>> Signed-off-by: Eric Blake <eblake@redhat.com> >>> > >>> +/* There are three classes of visitors; setting the class determines >>> + * how QAPI enums are visited, as well as what additional restrictions >>> + * can be asserted. */ >>> +typedef enum VisitorType { >>> + VISITOR_INPUT, >>> + VISITOR_OUTPUT, >>> + VISITOR_DEALLOC, >>> +} VisitorType; >>> + >>> struct Visitor >>> { >>> /* Must be set */ >> >> I think we should explain what makes a visitor an input/output/dealloc >> visitor. Not necessarily in this patch, and not necessarily in this >> place, just somewhere. Right now, the information is scattered. > > 8/19 might be the patch that does just that. We'll see what you think > when you get further through the review. > >>> @@ -514,16 +516,6 @@ opts_visitor_new(const QemuOpts *opts) >>> ov->visitor.next_list = &opts_next_list; >>> ov->visitor.end_list = &opts_end_list; >>> >>> - /* input_type_enum() covers both "normal" enums and union discriminators. >>> - * The union discriminator field is always generated as "type"; it should >>> - * match the "type" QemuOpt child of any QemuOpts. >>> - * >>> - * input_type_enum() will remove the looked-up key from the >>> - * "unprocessed_opts" hash even if the lookup fails, because the removal is >>> - * done earlier in opts_type_str(). This should be harmless. >>> - */ >>> - ov->visitor.type_enum = &input_type_enum; >>> - >> >> Hmm, this comment doesn't look worthless. With its statement gone, I >> guess it should move somewhere else. What do you think? > > The first half of the comment is fluff. The second half, about a > looked-up key being removed from unprocessed_opts even if lookup fails, > might be something I can move, but where? Maybe to the visit_type_enum() > in qapi-visit-core.c, stating that an input visitor will visit the > string even if conversion to enum fails? It really only affects what > happens for an input visitor that has a visit_check_struct() (commit > 14/19 of the series), but even then, we really only report an input > visit failure regarding unvisited options if there was no earlier error > - but the mere fact that visiting an enum type fails whether the string > was present but not a valid enum value, or whether the string was not > even present, means that we won't be reaching the visit_check_struct() > to even care about errors about unvisited members. > > Maybe that means I just move the documentation into the commit message, > and explain why the comment disappears (because a later patch will > guarantee the semantics that we only care about reporting unvisited > members in an input visitor only if all other visits are successful, so > it doesn't matter on earlier failure whether we consumed or did not > consume input). The second half of the comment points out that visiting an enum can have its side effect on unprocessed_opts even when the visit fails, namely when input_type_enum() fails after visit_type_str() succeded. Works as long as visit_type_str() has no "unwelcome" side effects. Your patch moves the enum handling into the visitor core. I think to replace the comment, we need two: 1. In the visitor implementation contract: state that visit_type_str() may be called for enums, and that the enum visit may fail even when visit_type_str() succeeds. No need to go into input vs. output detail, I think. 2. In opts_type_str(): point out that processed() gets called even though the visit may still fail if its an enum, but it should be harmless.
diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h index 2bd8f29..228a2a6 100644 --- a/include/qapi/visitor-impl.h +++ b/include/qapi/visitor-impl.h @@ -14,6 +14,15 @@ #include "qapi/visitor.h" +/* There are three classes of visitors; setting the class determines + * how QAPI enums are visited, as well as what additional restrictions + * can be asserted. */ +typedef enum VisitorType { + VISITOR_INPUT, + VISITOR_OUTPUT, + VISITOR_DEALLOC, +} VisitorType; + struct Visitor { /* Must be set */ @@ -36,10 +45,6 @@ struct Visitor void (*end_alternate)(Visitor *v); /* Must be set. */ - void (*type_enum)(Visitor *v, const char *name, int *obj, - const char *const strings[], Error **errp); - - /* Must be set. */ void (*type_int64)(Visitor *v, const char *name, int64_t *obj, Error **errp); /* Must be set. */ @@ -58,11 +63,9 @@ struct Visitor /* May be NULL; most useful for input visitors. */ void (*optional)(Visitor *v, const char *name, bool *present); + + /* Must be set. */ + VisitorType type; }; -void input_type_enum(Visitor *v, const char *name, int *obj, - const char *const strings[], Error **errp); -void output_type_enum(Visitor *v, const char *name, int *obj, - const char *const strings[], Error **errp); - #endif diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c index fa680c9..3cd7edc 100644 --- a/qapi/qapi-visit-core.c +++ b/qapi/qapi-visit-core.c @@ -72,12 +72,6 @@ bool visit_optional(Visitor *v, const char *name, bool *present) return *present; } -void visit_type_enum(Visitor *v, const char *name, int *obj, - const char *const strings[], Error **errp) -{ - v->type_enum(v, name, obj, strings, errp); -} - void visit_type_int(Visitor *v, const char *name, int64_t *obj, Error **errp) { v->type_int64(v, name, obj, errp); @@ -208,14 +202,13 @@ void visit_type_any(Visitor *v, const char *name, QObject **obj, Error **errp) v->type_any(v, name, obj, errp); } -void output_type_enum(Visitor *v, const char *name, int *obj, - const char *const strings[], Error **errp) +static void output_type_enum(Visitor *v, const char *name, int *obj, + const char *const strings[], Error **errp) { int i = 0; int value = *obj; char *enum_str; - assert(strings); while (strings[i++] != NULL); if (value < 0 || value >= i - 1) { error_setg(errp, QERR_INVALID_PARAMETER, name ? name : "null"); @@ -226,15 +219,13 @@ void output_type_enum(Visitor *v, const char *name, int *obj, visit_type_str(v, name, &enum_str, errp); } -void input_type_enum(Visitor *v, const char *name, int *obj, - const char *const strings[], Error **errp) +static void input_type_enum(Visitor *v, const char *name, int *obj, + const char *const strings[], Error **errp) { Error *local_err = NULL; int64_t value = 0; char *enum_str; - assert(strings); - visit_type_str(v, name, &enum_str, &local_err); if (local_err) { error_propagate(errp, local_err); @@ -257,3 +248,14 @@ void input_type_enum(Visitor *v, const char *name, int *obj, g_free(enum_str); *obj = value; } + +void visit_type_enum(Visitor *v, const char *name, int *obj, + const char *const strings[], Error **errp) +{ + assert(strings); + if (v->type == VISITOR_INPUT) { + input_type_enum(v, name, obj, strings, errp); + } else if (v->type == VISITOR_OUTPUT) { + output_type_enum(v, name, obj, strings, errp); + } +} diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c index 602f260..f98cf2e 100644 --- a/qapi/opts-visitor.c +++ b/qapi/opts-visitor.c @@ -507,6 +507,8 @@ opts_visitor_new(const QemuOpts *opts) ov = g_malloc0(sizeof *ov); + ov->visitor.type = VISITOR_INPUT; + ov->visitor.start_struct = &opts_start_struct; ov->visitor.end_struct = &opts_end_struct; @@ -514,16 +516,6 @@ opts_visitor_new(const QemuOpts *opts) ov->visitor.next_list = &opts_next_list; ov->visitor.end_list = &opts_end_list; - /* input_type_enum() covers both "normal" enums and union discriminators. - * The union discriminator field is always generated as "type"; it should - * match the "type" QemuOpt child of any QemuOpts. - * - * input_type_enum() will remove the looked-up key from the - * "unprocessed_opts" hash even if the lookup fails, because the removal is - * done earlier in opts_type_str(). This should be harmless. - */ - ov->visitor.type_enum = &input_type_enum; - ov->visitor.type_int64 = &opts_type_int64; ov->visitor.type_uint64 = &opts_type_uint64; ov->visitor.type_size = &opts_type_size; diff --git a/qapi/qapi-dealloc-visitor.c b/qapi/qapi-dealloc-visitor.c index 6922179..c19a459 100644 --- a/qapi/qapi-dealloc-visitor.c +++ b/qapi/qapi-dealloc-visitor.c @@ -163,11 +163,6 @@ static void qapi_dealloc_type_anything(Visitor *v, const char *name, } } -static void qapi_dealloc_type_enum(Visitor *v, const char *name, int *obj, - const char * const strings[], Error **errp) -{ -} - Visitor *qapi_dealloc_get_visitor(QapiDeallocVisitor *v) { return &v->visitor; @@ -184,6 +179,7 @@ QapiDeallocVisitor *qapi_dealloc_visitor_new(void) v = g_malloc0(sizeof(*v)); + v->visitor.type = VISITOR_DEALLOC; v->visitor.start_struct = qapi_dealloc_start_struct; v->visitor.end_struct = qapi_dealloc_end_struct; v->visitor.start_alternate = qapi_dealloc_start_alternate; @@ -191,7 +187,6 @@ QapiDeallocVisitor *qapi_dealloc_visitor_new(void) v->visitor.start_list = qapi_dealloc_start_list; v->visitor.next_list = qapi_dealloc_next_list; v->visitor.end_list = qapi_dealloc_end_list; - v->visitor.type_enum = qapi_dealloc_type_enum; v->visitor.type_int64 = qapi_dealloc_type_int64; v->visitor.type_uint64 = qapi_dealloc_type_uint64; v->visitor.type_bool = qapi_dealloc_type_bool; diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c index 7cd1b77..02d4233 100644 --- a/qapi/qmp-input-visitor.c +++ b/qapi/qmp-input-visitor.c @@ -339,13 +339,13 @@ QmpInputVisitor *qmp_input_visitor_new(QObject *obj) v = g_malloc0(sizeof(*v)); + v->visitor.type = VISITOR_INPUT; v->visitor.start_struct = qmp_input_start_struct; v->visitor.end_struct = qmp_input_end_struct; v->visitor.start_list = qmp_input_start_list; v->visitor.next_list = qmp_input_next_list; v->visitor.end_list = qmp_input_end_list; v->visitor.start_alternate = qmp_input_start_alternate; - v->visitor.type_enum = input_type_enum; v->visitor.type_int64 = qmp_input_type_int64; v->visitor.type_uint64 = qmp_input_type_uint64; v->visitor.type_bool = qmp_input_type_bool; diff --git a/qapi/qmp-output-visitor.c b/qapi/qmp-output-visitor.c index d44c676..1f2a7ba 100644 --- a/qapi/qmp-output-visitor.c +++ b/qapi/qmp-output-visitor.c @@ -234,12 +234,12 @@ QmpOutputVisitor *qmp_output_visitor_new(void) v = g_malloc0(sizeof(*v)); + v->visitor.type = VISITOR_OUTPUT; v->visitor.start_struct = qmp_output_start_struct; v->visitor.end_struct = qmp_output_end_struct; v->visitor.start_list = qmp_output_start_list; v->visitor.next_list = qmp_output_next_list; v->visitor.end_list = qmp_output_end_list; - v->visitor.type_enum = output_type_enum; v->visitor.type_int64 = qmp_output_type_int64; v->visitor.type_uint64 = qmp_output_type_uint64; v->visitor.type_bool = qmp_output_type_bool; diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c index ab12953..d604575 100644 --- a/qapi/string-input-visitor.c +++ b/qapi/string-input-visitor.c @@ -348,7 +348,7 @@ StringInputVisitor *string_input_visitor_new(const char *str) v = g_malloc0(sizeof(*v)); - v->visitor.type_enum = input_type_enum; + v->visitor.type = VISITOR_INPUT; v->visitor.type_int64 = parse_type_int64; v->visitor.type_uint64 = parse_type_uint64; v->visitor.type_size = parse_type_size; diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c index c2e5c5b..0d44d7e 100644 --- a/qapi/string-output-visitor.c +++ b/qapi/string-output-visitor.c @@ -351,7 +351,7 @@ StringOutputVisitor *string_output_visitor_new(bool human) v->string = g_string_new(NULL); v->human = human; - v->visitor.type_enum = output_type_enum; + v->visitor.type = VISITOR_OUTPUT; v->visitor.type_int64 = print_type_int64; v->visitor.type_uint64 = print_type_uint64; v->visitor.type_size = print_type_size;
We have three classes of QAPI visitors: input, output, and dealloc. Currently, all implementations of these visitors have one thing in common based on their visitor type: the implementation used for the visit_type_enum() callback. But since we plan to add more such common behavior, in relation to documenting and further refining the semantics, it makes more sense to have the visitor implementations advertise which class they belong to, so the common qapi-visit-core code can use that information in multiple places. For this patch, knowing the class of a visitor implementation lets us make input_type_enum() and output_type_enum() become static functions, by replacing the callback function Visitor.type_enum() with the simpler enum member Visitor.type. Share a common assertion in qapi-visit-core as part of the refactoring. Signed-off-by: Eric Blake <eblake@redhat.com> --- v14: no change v13: no change v12: new patch --- include/qapi/visitor-impl.h | 21 ++++++++++++--------- qapi/qapi-visit-core.c | 28 +++++++++++++++------------- qapi/opts-visitor.c | 12 ++---------- qapi/qapi-dealloc-visitor.c | 7 +------ qapi/qmp-input-visitor.c | 2 +- qapi/qmp-output-visitor.c | 2 +- qapi/string-input-visitor.c | 2 +- qapi/string-output-visitor.c | 2 +- 8 files changed, 34 insertions(+), 42 deletions(-)