Message ID | 1444710158-8723-11-git-send-email-eblake@redhat.com |
---|---|
State | New |
Headers | show |
Eric Blake <eblake@redhat.com> writes: > Right now, simple unions have a quirk of using 'kind' in the C > struct to match the QMP wire name 'type'. This has resulted in > messy clients each doing special cases. While we plan to > eventually rename things to match, it is better in the meantime > to consolidate the quirks into a special subclass, by adding a > new member.c_name() function. This will also make it easier > for reworking how alternate types are laid out in a future > patch. Use the new c_name() function where possible. Terminology: "the new c_name() method". I don't like having both function c_name() and method c_name(), because it's very easy to use the function when you should use the method, and the mistakes will make things break only rarely, so they can go undetected easily. I'm okay with this patch only because you assure me the whole thing is temporary: "# TODO Drop this class once we no longer have the 'type'/'kind' mismatch". > No change to generated code. > > Signed-off-by: Eric Blake <eblake@redhat.com> > > --- v8: use + instead of interpolation in a few places, rebase to simpler .is_implicit(), use .c_name() more. > v7: new patch, but borrows idea of subclass from v6 10/12, as > well as c_name() from 7/12 > --- > scripts/qapi-commands.py | 8 ++++---- > scripts/qapi-types.py | 12 +++++------- > scripts/qapi-visit.py | 17 +++++------------ > scripts/qapi.py | 26 +++++++++++++++++++------- > 4 files changed, 33 insertions(+), 30 deletions(-) > > diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py > index 43a893b..53a79ab 100644 > --- a/scripts/qapi-commands.py > +++ b/scripts/qapi-commands.py > @@ -32,8 +32,8 @@ def gen_call(name, arg_type, ret_type): > if arg_type: > for memb in arg_type.members: > if memb.optional: > - argstr += 'has_%s, ' % c_name(memb.name) > - argstr += '%s, ' % c_name(memb.name) > + argstr += 'has_' + memb.c_name() + ', ' > + argstr += memb.c_name() + ', ' I like to use + instead of % in sufficiently simple cases myself. Not sure this is one. See also change to gen_params() below. > > lhs = '' > if ret_type: > @@ -77,11 +77,11 @@ def gen_marshal_vars(arg_type, ret_type): > ret += mcgen(''' > bool has_%(c_name)s = false; > ''', > - c_name=c_name(memb.name)) > + c_name=memb.c_name()) > ret += mcgen(''' > %(c_type)s %(c_name)s = %(c_null)s; > ''', > - c_name=c_name(memb.name), > + c_name=memb.c_name(), > c_type=memb.type.c_type(), > c_null=memb.type.c_null()) > ret += '\n' > diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py > index 4fe618e..e37d529 100644 > --- a/scripts/qapi-types.py > +++ b/scripts/qapi-types.py > @@ -136,9 +136,10 @@ struct %(c_name)s { > ''') > else: > ret += mcgen(''' > - %(c_type)s kind; > + %(c_type)s %(c_name)s; > ''', > - c_type=c_name(variants.tag_member.type.name)) > + c_type=variants.tag_member.type.c_name(), > + c_name=variants.tag_member.c_name()) > > # FIXME: What purpose does data serve, besides preventing a union that > # has a branch named 'data'? We use it in qapi-visit.py to decide > @@ -152,10 +153,7 @@ struct %(c_name)s { > union { /* union tag is @%(c_name)s */ > void *data; > ''', > - # TODO ugly special case for simple union > - # Use same tag name in C as on the wire to get rid of > - # it, then: c_name=c_name(variants.tag_member.name) > - c_name=c_name(variants.tag_name or 'kind')) > + c_name=variants.tag_member.c_name()) > > for var in variants.variants: > # Ugly special case for simple union TODO get rid of it > @@ -164,7 +162,7 @@ struct %(c_name)s { > %(c_type)s %(c_name)s; > ''', > c_type=typ.c_type(), > - c_name=c_name(var.name)) > + c_name=var.c_name()) > > ret += mcgen(''' > }; > diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py > index 2a9fab8..cb251b2 100644 > --- a/scripts/qapi-visit.py > +++ b/scripts/qapi-visit.py > @@ -196,7 +196,7 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error > case=c_enum_const(variants.tag_member.type.name, > var.name), > c_type=var.type.c_name(), > - c_name=c_name(var.name)) > + c_name=var.c_name()) > > ret += mcgen(''' > default: > @@ -249,10 +249,6 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error > c_name=c_name(name)) > ret += gen_err_check(label='out_obj') > > - tag_key = variants.tag_member.name > - if not variants.tag_name: > - # we pointlessly use a different key for simple unions > - tag_key = 'type' > ret += mcgen(''' > visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "%(name)s", &err); > if (err) { > @@ -264,11 +260,8 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error > switch ((*obj)->%(c_name)s) { > ''', > c_type=variants.tag_member.type.c_name(), > - # TODO ugly special case for simple union > - # Use same tag name in C as on the wire to get rid of > - # it, then: c_name=c_name(variants.tag_member.name) > - c_name=c_name(variants.tag_name or 'kind'), > - name=tag_key) > + c_name=variants.tag_member.c_name(), > + name=variants.tag_member.name) > > for var in variants.variants: > # TODO ugly special case for simple union > @@ -283,13 +276,13 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error > visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "data", &err); > ''', > c_type=simple_union_type.c_name(), > - c_name=c_name(var.name)) > + c_name=var.c_name()) > else: > ret += mcgen(''' > visit_type_implicit_%(c_type)s(v, &(*obj)->%(c_name)s, &err); > ''', > c_type=var.type.c_name(), > - c_name=c_name(var.name)) > + c_name=var.c_name()) > ret += mcgen(''' > break; > ''') > diff --git a/scripts/qapi.py b/scripts/qapi.py > index 5b66264..80c026b 100644 > --- a/scripts/qapi.py > +++ b/scripts/qapi.py > @@ -975,7 +975,7 @@ class QAPISchemaObjectType(QAPISchemaType): > members = [] > seen = {} > for m in members: > - assert c_name(m.name) not in seen > + assert m.c_name() not in seen > seen[m.name] = m > for m in self.local_members: > m.check(schema, members, seen) > @@ -1022,6 +1022,18 @@ class QAPISchemaObjectTypeMember(object): > all_members.append(self) > seen[self.name] = self > > + def c_name(self): > + return c_name(self.name) > + > + > +# TODO Drop this class once we no longer have the 'type'/'kind' mismatch > +class QAPISchemaObjectTypeUnionTag(QAPISchemaObjectTypeMember): > + def c_name(self): > + assert self.name == 'type' > + assert isinstance(self.type, QAPISchemaEnumType) > + assert self.type.is_implicit() > + return 'kind' > + > > class QAPISchemaObjectTypeVariants(object): > def __init__(self, tag_name, tag_member, variants): > @@ -1248,7 +1260,7 @@ class QAPISchema(object): > def _make_implicit_tag(self, type_name, variants): > typ = self._make_implicit_enum_type(type_name, > [v.name for v in variants]) > - return QAPISchemaObjectTypeMember('type', typ, False) > + return QAPISchemaObjectTypeUnionTag('type', typ, False) > > def _def_union_type(self, expr, info): > name = expr['union'] > @@ -1555,8 +1567,8 @@ def gen_params(arg_type, extra): > ret += sep > sep = ', ' > if memb.optional: > - ret += 'bool has_%s, ' % c_name(memb.name) > - ret += '%s %s' % (memb.type.c_type(is_param=True), c_name(memb.name)) > + ret += 'bool has_' + memb.c_name() + sep > + ret += '%s %s' % (memb.type.c_type(is_param=True), memb.c_name()) > if extra: > ret += sep + extra > return ret New hunk in v8, to remain consistent with gen_call(). I doubt replacing literal ', ' by sep is making things clearer. > @@ -1585,13 +1597,13 @@ def gen_visit_fields(members, prefix='', need_cast=False, skiperr=False): > ret += mcgen(''' > visit_optional(v, &%(prefix)shas_%(c_name)s, "%(name)s", %(errp)s); > ''', > - prefix=prefix, c_name=c_name(memb.name), > + prefix=prefix, c_name=memb.c_name(), > name=memb.name, errp=errparg) > ret += gen_err_check(skiperr=skiperr) > ret += mcgen(''' > if (%(prefix)shas_%(c_name)s) { > ''', > - prefix=prefix, c_name=c_name(memb.name)) > + prefix=prefix, c_name=memb.c_name()) > push_indent() > > # Ugly: sometimes we need to cast away const > @@ -1604,7 +1616,7 @@ def gen_visit_fields(members, prefix='', need_cast=False, skiperr=False): > visit_type_%(c_type)s(v, %(cast)s&%(prefix)s%(c_name)s, "%(name)s", %(errp)s); > ''', > c_type=memb.type.c_name(), prefix=prefix, cast=cast, > - c_name=c_name(memb.name), name=memb.name, > + c_name=memb.c_name(), name=memb.name, > errp=errparg) > ret += gen_err_check(skiperr=skiperr) New hunks in v8. Do you change from function c_name() to method .c_name() Just for consistency, or is there a more serious reason?
On 10/13/2015 06:10 AM, Markus Armbruster wrote: > Eric Blake <eblake@redhat.com> writes: > >> Right now, simple unions have a quirk of using 'kind' in the C >> struct to match the QMP wire name 'type'. This has resulted in >> messy clients each doing special cases. While we plan to >> eventually rename things to match, it is better in the meantime >> to consolidate the quirks into a special subclass, by adding a >> new member.c_name() function. This will also make it easier >> for reworking how alternate types are laid out in a future >> patch. Use the new c_name() function where possible. > > Terminology: "the new c_name() method". > > I don't like having both function c_name() and method c_name(), because > it's very easy to use the function when you should use the method, and > the mistakes will make things break only rarely, so they can go > undetected easily. I'm okay with this patch only because you assure me > the whole thing is temporary: "# TODO Drop this class once we no longer > have the 'type'/'kind' mismatch". Hmm. Even after my type/kind fix is in, my local tree doesn't (yet) remove uses of c_name(), because of its shorter typing. But you are correct that once the rename is in and the temporary QAPISchemaObjectTypeUnionTag is deleted, then there is no longer a difference between member.c_name() and the longer c_name(member.name). On the other hand, my patch subset C adds a member.c_type() function which is required for use on simplified alternate layout, because it is not always the same as member.type.c_type() or c_type(member.type.name). As it is, we already have cases where c_name(entity.name) is not the same as entity.c_name(), so we already have the confusion of when to use the global function vs. the member function. Is it worth renaming things so that the global function and member functions have different names? If so, which one would be renamed, and to what? > >> No change to generated code. >> >> Signed-off-by: Eric Blake <eblake@redhat.com> >> >> --- > > v8: use + instead of interpolation in a few places, rebase to simpler > .is_implicit(), use .c_name() more. Whoops, forgot to 'git commit --amend' this one. Looks like you are also viewing interdiffs, though, which makes me feel better about how the series is progressing. >> +++ b/scripts/qapi-commands.py >> @@ -32,8 +32,8 @@ def gen_call(name, arg_type, ret_type): >> if arg_type: >> for memb in arg_type.members: >> if memb.optional: >> - argstr += 'has_%s, ' % c_name(memb.name) >> - argstr += '%s, ' % c_name(memb.name) >> + argstr += 'has_' + memb.c_name() + ', ' >> + argstr += memb.c_name() + ', ' > > I like to use + instead of % in sufficiently simple cases myself. Not > sure this is one. See also change to gen_params() below. >> @@ -1555,8 +1567,8 @@ def gen_params(arg_type, extra): >> ret += sep >> sep = ', ' >> if memb.optional: >> - ret += 'bool has_%s, ' % c_name(memb.name) >> - ret += '%s %s' % (memb.type.c_type(is_param=True), c_name(memb.name)) >> + ret += 'bool has_' + memb.c_name() + sep >> + ret += '%s %s' % (memb.type.c_type(is_param=True), memb.c_name()) >> if extra: >> ret += sep + extra >> return ret > > New hunk in v8, to remain consistent with gen_call(). > > I doubt replacing literal ', ' by sep is making things clearer. Fair enough - if there is reason for respin, I can undo the changes to using '+'. >> @@ -1604,7 +1616,7 @@ def gen_visit_fields(members, prefix='', need_cast=False, skiperr=False): >> visit_type_%(c_type)s(v, %(cast)s&%(prefix)s%(c_name)s, "%(name)s", %(errp)s); >> ''', >> c_type=memb.type.c_name(), prefix=prefix, cast=cast, >> - c_name=c_name(memb.name), name=memb.name, >> + c_name=memb.c_name(), name=memb.name, >> errp=errparg) >> ret += gen_err_check(skiperr=skiperr) > > New hunks in v8. Do you change from function c_name() to method > .c_name() Just for consistency, or is there a more serious reason? It matters the most in qapi-type's gen_union(); everywhere else, it is just for consistency and less typing. What is the plan of attack on this one - will I need to respin a v9?
Eric Blake <eblake@redhat.com> writes: > On 10/13/2015 06:10 AM, Markus Armbruster wrote: >> Eric Blake <eblake@redhat.com> writes: >> >>> Right now, simple unions have a quirk of using 'kind' in the C >>> struct to match the QMP wire name 'type'. This has resulted in >>> messy clients each doing special cases. While we plan to >>> eventually rename things to match, it is better in the meantime >>> to consolidate the quirks into a special subclass, by adding a >>> new member.c_name() function. This will also make it easier >>> for reworking how alternate types are laid out in a future >>> patch. Use the new c_name() function where possible. >> >> Terminology: "the new c_name() method". >> >> I don't like having both function c_name() and method c_name(), because >> it's very easy to use the function when you should use the method, and >> the mistakes will make things break only rarely, so they can go >> undetected easily. I'm okay with this patch only because you assure me >> the whole thing is temporary: "# TODO Drop this class once we no longer >> have the 'type'/'kind' mismatch". > > Hmm. Even after my type/kind fix is in, my local tree doesn't (yet) > remove uses of c_name(), because of its shorter typing. But you are > correct that once the rename is in and the temporary > QAPISchemaObjectTypeUnionTag is deleted, then there is no longer a > difference between member.c_name() and the longer c_name(member.name). > > On the other hand, my patch subset C adds a member.c_type() function > which is required for use on simplified alternate layout, because it is > not always the same as member.type.c_type() or c_type(member.type.name). Can't say how I like it until I reviewed it :) > As it is, we already have cases where c_name(entity.name) is not the > same as entity.c_name(), so we already have the confusion of when to use > the global function vs. the member function. They are: * QAPISchemaBuiltinType.c_name() returns its argument instead * QAPISchemaObjectType.c_name() additionally asserts "not implicit". * QAPISchemaObjectTypeUnionTag.c_name() returns 'kind' instead, but that'll go away. Anything else? > Is it worth renaming things so that the global function and member > functions have different names? If so, which one would be renamed, and > to what? Renaming one of them can perhaps make misuse of the function stand out a bit more. The only way I can see to keep obvious use of the interface correct is getting rid of the function entirely. Involves passing objects instead of names around, then calling the objects' method instead of passing the name to the function. Can't say whether a suitable object always exists without trying it. >>> No change to generated code. >>> >>> Signed-off-by: Eric Blake <eblake@redhat.com> >>> >>> --- >> >> v8: use + instead of interpolation in a few places, rebase to simpler >> .is_implicit(), use .c_name() more. > > Whoops, forgot to 'git commit --amend' this one. Looks like you are > also viewing interdiffs, though, which makes me feel better about how > the series is progressing. When I expect only small and scattered change, I like to feed patches to ediff :) >>> +++ b/scripts/qapi-commands.py >>> @@ -32,8 +32,8 @@ def gen_call(name, arg_type, ret_type): >>> if arg_type: >>> for memb in arg_type.members: >>> if memb.optional: >>> - argstr += 'has_%s, ' % c_name(memb.name) >>> - argstr += '%s, ' % c_name(memb.name) >>> + argstr += 'has_' + memb.c_name() + ', ' >>> + argstr += memb.c_name() + ', ' >> >> I like to use + instead of % in sufficiently simple cases myself. Not >> sure this is one. See also change to gen_params() below. > >>> @@ -1555,8 +1567,8 @@ def gen_params(arg_type, extra): >>> ret += sep >>> sep = ', ' >>> if memb.optional: >>> - ret += 'bool has_%s, ' % c_name(memb.name) >>> - ret += '%s %s' % (memb.type.c_type(is_param=True), c_name(memb.name)) >>> + ret += 'bool has_' + memb.c_name() + sep >>> + ret += '%s %s' % (memb.type.c_type(is_param=True), memb.c_name()) >>> if extra: >>> ret += sep + extra >>> return ret >> >> New hunk in v8, to remain consistent with gen_call(). >> >> I doubt replacing literal ', ' by sep is making things clearer. > > Fair enough - if there is reason for respin, I can undo the changes to > using '+'. > >>> @@ -1604,7 +1616,7 @@ def gen_visit_fields(members, prefix='', need_cast=False, skiperr=False): >>> visit_type_%(c_type)s(v, %(cast)s&%(prefix)s%(c_name)s, "%(name)s", %(errp)s); >>> ''', >>> c_type=memb.type.c_name(), prefix=prefix, cast=cast, >>> - c_name=c_name(memb.name), name=memb.name, >>> + c_name=memb.c_name(), name=memb.name, >>> errp=errparg) >>> ret += gen_err_check(skiperr=skiperr) >> >> New hunks in v8. Do you change from function c_name() to method >> .c_name() Just for consistency, or is there a more serious reason? > > It matters the most in qapi-type's gen_union(); everywhere else, it is > just for consistency and less typing. > > What is the plan of attack on this one - will I need to respin a v9? I'll answer this in my reply to PATCH 15.
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py index 43a893b..53a79ab 100644 --- a/scripts/qapi-commands.py +++ b/scripts/qapi-commands.py @@ -32,8 +32,8 @@ def gen_call(name, arg_type, ret_type): if arg_type: for memb in arg_type.members: if memb.optional: - argstr += 'has_%s, ' % c_name(memb.name) - argstr += '%s, ' % c_name(memb.name) + argstr += 'has_' + memb.c_name() + ', ' + argstr += memb.c_name() + ', ' lhs = '' if ret_type: @@ -77,11 +77,11 @@ def gen_marshal_vars(arg_type, ret_type): ret += mcgen(''' bool has_%(c_name)s = false; ''', - c_name=c_name(memb.name)) + c_name=memb.c_name()) ret += mcgen(''' %(c_type)s %(c_name)s = %(c_null)s; ''', - c_name=c_name(memb.name), + c_name=memb.c_name(), c_type=memb.type.c_type(), c_null=memb.type.c_null()) ret += '\n' diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py index 4fe618e..e37d529 100644 --- a/scripts/qapi-types.py +++ b/scripts/qapi-types.py @@ -136,9 +136,10 @@ struct %(c_name)s { ''') else: ret += mcgen(''' - %(c_type)s kind; + %(c_type)s %(c_name)s; ''', - c_type=c_name(variants.tag_member.type.name)) + c_type=variants.tag_member.type.c_name(), + c_name=variants.tag_member.c_name()) # FIXME: What purpose does data serve, besides preventing a union that # has a branch named 'data'? We use it in qapi-visit.py to decide @@ -152,10 +153,7 @@ struct %(c_name)s { union { /* union tag is @%(c_name)s */ void *data; ''', - # TODO ugly special case for simple union - # Use same tag name in C as on the wire to get rid of - # it, then: c_name=c_name(variants.tag_member.name) - c_name=c_name(variants.tag_name or 'kind')) + c_name=variants.tag_member.c_name()) for var in variants.variants: # Ugly special case for simple union TODO get rid of it @@ -164,7 +162,7 @@ struct %(c_name)s { %(c_type)s %(c_name)s; ''', c_type=typ.c_type(), - c_name=c_name(var.name)) + c_name=var.c_name()) ret += mcgen(''' }; diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index 2a9fab8..cb251b2 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -196,7 +196,7 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error case=c_enum_const(variants.tag_member.type.name, var.name), c_type=var.type.c_name(), - c_name=c_name(var.name)) + c_name=var.c_name()) ret += mcgen(''' default: @@ -249,10 +249,6 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error c_name=c_name(name)) ret += gen_err_check(label='out_obj') - tag_key = variants.tag_member.name - if not variants.tag_name: - # we pointlessly use a different key for simple unions - tag_key = 'type' ret += mcgen(''' visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "%(name)s", &err); if (err) { @@ -264,11 +260,8 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error switch ((*obj)->%(c_name)s) { ''', c_type=variants.tag_member.type.c_name(), - # TODO ugly special case for simple union - # Use same tag name in C as on the wire to get rid of - # it, then: c_name=c_name(variants.tag_member.name) - c_name=c_name(variants.tag_name or 'kind'), - name=tag_key) + c_name=variants.tag_member.c_name(), + name=variants.tag_member.name) for var in variants.variants: # TODO ugly special case for simple union @@ -283,13 +276,13 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "data", &err); ''', c_type=simple_union_type.c_name(), - c_name=c_name(var.name)) + c_name=var.c_name()) else: ret += mcgen(''' visit_type_implicit_%(c_type)s(v, &(*obj)->%(c_name)s, &err); ''', c_type=var.type.c_name(), - c_name=c_name(var.name)) + c_name=var.c_name()) ret += mcgen(''' break; ''') diff --git a/scripts/qapi.py b/scripts/qapi.py index 5b66264..80c026b 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -975,7 +975,7 @@ class QAPISchemaObjectType(QAPISchemaType): members = [] seen = {} for m in members: - assert c_name(m.name) not in seen + assert m.c_name() not in seen seen[m.name] = m for m in self.local_members: m.check(schema, members, seen) @@ -1022,6 +1022,18 @@ class QAPISchemaObjectTypeMember(object): all_members.append(self) seen[self.name] = self + def c_name(self): + return c_name(self.name) + + +# TODO Drop this class once we no longer have the 'type'/'kind' mismatch +class QAPISchemaObjectTypeUnionTag(QAPISchemaObjectTypeMember): + def c_name(self): + assert self.name == 'type' + assert isinstance(self.type, QAPISchemaEnumType) + assert self.type.is_implicit() + return 'kind' + class QAPISchemaObjectTypeVariants(object): def __init__(self, tag_name, tag_member, variants): @@ -1248,7 +1260,7 @@ class QAPISchema(object): def _make_implicit_tag(self, type_name, variants): typ = self._make_implicit_enum_type(type_name, [v.name for v in variants]) - return QAPISchemaObjectTypeMember('type', typ, False) + return QAPISchemaObjectTypeUnionTag('type', typ, False) def _def_union_type(self, expr, info): name = expr['union'] @@ -1555,8 +1567,8 @@ def gen_params(arg_type, extra): ret += sep sep = ', ' if memb.optional: - ret += 'bool has_%s, ' % c_name(memb.name) - ret += '%s %s' % (memb.type.c_type(is_param=True), c_name(memb.name)) + ret += 'bool has_' + memb.c_name() + sep + ret += '%s %s' % (memb.type.c_type(is_param=True), memb.c_name()) if extra: ret += sep + extra return ret @@ -1585,13 +1597,13 @@ def gen_visit_fields(members, prefix='', need_cast=False, skiperr=False): ret += mcgen(''' visit_optional(v, &%(prefix)shas_%(c_name)s, "%(name)s", %(errp)s); ''', - prefix=prefix, c_name=c_name(memb.name), + prefix=prefix, c_name=memb.c_name(), name=memb.name, errp=errparg) ret += gen_err_check(skiperr=skiperr) ret += mcgen(''' if (%(prefix)shas_%(c_name)s) { ''', - prefix=prefix, c_name=c_name(memb.name)) + prefix=prefix, c_name=memb.c_name()) push_indent() # Ugly: sometimes we need to cast away const @@ -1604,7 +1616,7 @@ def gen_visit_fields(members, prefix='', need_cast=False, skiperr=False): visit_type_%(c_type)s(v, %(cast)s&%(prefix)s%(c_name)s, "%(name)s", %(errp)s); ''', c_type=memb.type.c_name(), prefix=prefix, cast=cast, - c_name=c_name(memb.name), name=memb.name, + c_name=memb.c_name(), name=memb.name, errp=errparg) ret += gen_err_check(skiperr=skiperr)
Right now, simple unions have a quirk of using 'kind' in the C struct to match the QMP wire name 'type'. This has resulted in messy clients each doing special cases. While we plan to eventually rename things to match, it is better in the meantime to consolidate the quirks into a special subclass, by adding a new member.c_name() function. This will also make it easier for reworking how alternate types are laid out in a future patch. Use the new c_name() function where possible. No change to generated code. Signed-off-by: Eric Blake <eblake@redhat.com> --- v7: new patch, but borrows idea of subclass from v6 10/12, as well as c_name() from 7/12 --- scripts/qapi-commands.py | 8 ++++---- scripts/qapi-types.py | 12 +++++------- scripts/qapi-visit.py | 17 +++++------------ scripts/qapi.py | 26 +++++++++++++++++++------- 4 files changed, 33 insertions(+), 30 deletions(-)