| Message ID | 1427995743-7865-3-git-send-email-armbru@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 04/02/2015 11:28 AM, Markus Armbruster wrote: > c_fun() maps '.' to '_', c_var() doesn't. Nothing prevents '.' in > QAPI names that get passed to c_var(). > > Which QAPI names get passed to c_fun(), to c_var(), or to both is not > obvious. Names of command parameters and struct type members get > passed to c_var(). > > c_var() strips a leading '*', but this cannot happen. c_fun() > doesn't. > > Fix c_var() to work exactly like c_fun(). > > Perhaps they should be replaced by a single mapping function. How much harder is that to do? Also, this commit probably means my qapi testsuite enhancements ought to add tests that we support downstream extensions (__name.name_blah) in a variety of situations. But I'm merely adding it to my (growing) list of post-series additions, and won't hold up v6 adding it. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > scripts/qapi.py | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) If it's not too hard to use a single mapping function, that might look prettier. But from the raw perspective of fixing an inconsistency in the code: Reviewed-by: Eric Blake <eblake@redhat.com>
Eric Blake <eblake@redhat.com> writes: > On 04/02/2015 11:28 AM, Markus Armbruster wrote: >> c_fun() maps '.' to '_', c_var() doesn't. Nothing prevents '.' in >> QAPI names that get passed to c_var(). >> >> Which QAPI names get passed to c_fun(), to c_var(), or to both is not >> obvious. Names of command parameters and struct type members get >> passed to c_var(). >> >> c_var() strips a leading '*', but this cannot happen. c_fun() >> doesn't. >> >> Fix c_var() to work exactly like c_fun(). >> >> Perhaps they should be replaced by a single mapping function. > > How much harder is that to do? Not hard at all. I refrained from doing it for another reason, namely that it would remove "variable" vs. "function" information from the source. This distinction may not be useful, and whether it's actually done consistently in the source is even more doubtful, but I didn't wish to decide that when I wrote the patch. You're pursuing it in your "Fix C identifiers generated for names containing '.'" series now. I'll review. > Also, this commit probably means my qapi testsuite enhancements ought to > add tests that we support downstream extensions (__name.name_blah) in a > variety of situations. But I'm merely adding it to my (growing) list of > post-series additions, and won't hold up v6 adding it. > >> >> Signed-off-by: Markus Armbruster <armbru@redhat.com> >> --- >> scripts/qapi.py | 6 ++++-- >> 1 file changed, 4 insertions(+), 2 deletions(-) > > If it's not too hard to use a single mapping function, that might look > prettier. But from the raw perspective of fixing an inconsistency in > the code: > > Reviewed-by: Eric Blake <eblake@redhat.com> Thanks!
diff --git a/scripts/qapi.py b/scripts/qapi.py index 8e5b4ad..6d102df 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -693,6 +693,8 @@ def camel_case(name): new_name += ch.lower() return new_name +c_var_trans = string.maketrans('.-', '__') + def c_var(name, protect=True): # ANSI X3J11/88-090, 3.1.1 c89_words = set(['auto', 'break', 'case', 'char', 'const', 'continue', @@ -722,10 +724,10 @@ def c_var(name, protect=True): polluted_words = set(['unix', 'errno']) if protect and (name in c89_words | c99_words | c11_words | gcc_words | cpp_words | polluted_words): return "q_" + name - return name.replace('-', '_').lstrip("*") + return name.translate(c_var_trans) def c_fun(name, protect=True): - return c_var(name, protect).replace('.', '_') + return c_var(name, protect) def c_list_type(name): return '%sList' % name
c_fun() maps '.' to '_', c_var() doesn't. Nothing prevents '.' in QAPI names that get passed to c_var(). Which QAPI names get passed to c_fun(), to c_var(), or to both is not obvious. Names of command parameters and struct type members get passed to c_var(). c_var() strips a leading '*', but this cannot happen. c_fun() doesn't. Fix c_var() to work exactly like c_fun(). Perhaps they should be replaced by a single mapping function. Signed-off-by: Markus Armbruster <armbru@redhat.com> --- scripts/qapi.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)