[Spice-devel] [PATCH spice-common 4/9] codegen: Allows to generate C declarations automatically
Frediano Ziglio
fziglio at redhat.com
Fri Mar 8 21:47:35 UTC 2019
>
> On Sun, Mar 03, 2019 at 07:10:25PM +0000, Frediano Ziglio wrote:
> > Allows to specify a @declare attribute for messages and structure
> > that can generate the needed C structures.
> >
> > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > ---
> > python_modules/ptypes.py | 64 ++++++++++++++++++++++++++++++++++++++++
> > spice_codegen.py | 47 +++++++++++++++++++++++++++++
> > 2 files changed, 111 insertions(+)
> >
> > diff --git a/python_modules/ptypes.py b/python_modules/ptypes.py
> > index 7dca78d..64c198e 100644
> > --- a/python_modules/ptypes.py
> > +++ b/python_modules/ptypes.py
> > @@ -72,6 +72,8 @@ valid_attributes=set([
> > 'zero',
> > # this attribute does not exist on the network, fill just structure
> > with the value
> > 'virtual',
> > + # generate C structure declarations from protocol definition
> > + 'declare',
> > ])
> >
> > attributes_with_arguments=set([
> > @@ -485,6 +487,26 @@ class ArrayType(Type):
> > def c_type(self):
> > return self.element_type.c_type()
> >
> > + def generate_c_declaration(self, writer, member):
> > + name = member.name
> > + if member.has_attr("chunk"):
> > + return writer.writeln('SpiceChunks *%s;' % name)
> > + if member.has_attr("as_ptr"):
> > + len_var = member.attributes["as_ptr"][0]
> > + writer.writeln('uint32_t %s;' % len_var)
> > + return writer.writeln('%s *%s;' % (self.c_type(), name))
> > + if member.has_attr("to_ptr"): # TODO len
> > + return writer.writeln('%s *%s;' % (self.c_type(), name))
> > + if member.has_attr("ptr_array"): # TODO where the length is
> > stored? overflow?
> > + return writer.writeln('%s *%s[0];' % (self.c_type(), name))
> > + if member.has_end_attr() or self.is_remaining_length(): # TODO len
> > + return writer.writeln('%s %s[0];' % (self.c_type(), name))
>
> These TODO are worrying, but only the has_end_attr() one seems to be run
> at the moment, the others seem to be dead code?
>
Indeed. And also is more a "for security we should avoid that in the
protocol" so it's not much related to code generation.
The idea is that "if you have an array as output you should also have
the length in the output otherwise you are probably generating an
overflow".
The check should be done parsing the protocol file, not generating
a specific code from it (surely demarshaller code is affected).
Maybe the best is to move these TODOs in a separate patch.
And possibly I'll keep it somewhere when I'll have the time to
generate the proper checks.
> Looks good otherwise,
>
> Acked-by: Christophe Fergeau <cfergeau at redhat.com>
>
> > + if self.is_constant_length():
> > + return writer.writeln('%s %s[%s];' % (self.c_type(), name,
> > self.size))
> > + if self.is_identifier_length():
> > + return writer.writeln('%s *%s;' % (self.c_type(), name))
> > + raise NotImplementedError('unknown array %s' % str(self))
> > +
> > class PointerType(Type):
> > def __init__(self, target_type):
> > Type.__init__(self)
> > @@ -529,6 +551,15 @@ class PointerType(Type):
> > def get_num_pointers(self):
> > return 1
> >
> > + def generate_c_declaration(self, writer, member):
> > + target_type = self.target_type
> > + is_array = target_type.is_array()
> > + if not is_array or target_type.is_identifier_length():
> > + writer.writeln("%s *%s;" % (target_type.c_type(),
> > member.name))
> > + return
> > + raise NotImplementedError('Some pointers to array declarations are
> > not implemented %s' %
> > +member)
> > +
> > class Containee:
> > def __init__(self):
> > self.attributes = {}
> > @@ -624,6 +655,14 @@ class Member(Containee):
> > names = [prefix + "_" + name for name in names]
> > return names
> >
> > + def generate_c_declaration(self, writer):
> > + if self.has_attr("zero"):
> > + return
> > + if self.is_pointer() or self.is_array():
> > + self.member_type.generate_c_declaration(writer, self)
> > + return
> > + return writer.writeln("%s %s;" % (self.member_type.c_type(),
> > self.name))
> > +
> > class SwitchCase:
> > def __init__(self, values, member):
> > self.values = values
> > @@ -747,6 +786,17 @@ class Switch(Containee):
> > names = names + c.get_pointer_names(marshalled)
> > return names
> >
> > + def generate_c_declaration(self, writer):
> > + if self.has_attr("anon") and len(self.cases) == 1:
> > + self.cases[0].member.generate_c_declaration(writer)
> > + return
> > + writer.writeln('union {')
> > + writer.indent()
> > + for m in self.cases:
> > + m.member.generate_c_declaration(writer)
> > + writer.unindent()
> > + writer.writeln('} %s;' % self.name)
> > +
> > class ContainerType(Type):
> > def is_fixed_sizeof(self):
> > for m in self.members:
> > @@ -857,6 +907,20 @@ class ContainerType(Type):
> >
> > return member
> >
> > + def generate_c_declaration(self, writer):
> > + if not self.has_attr('declare'):
> > + return
> > + name = self.c_type()
> > + writer.writeln('typedef struct %s {' % name)
> > + writer.indent()
> > + for m in self.members:
> > + m.generate_c_declaration(writer)
> > + if len(self.members) == 0:
> > + # make sure generated structure are not empty
> > + writer.writeln("char dummy[0];")
> > + writer.unindent()
> > + writer.writeln('} %s;' % name).newline()
> > +
> > class StructType(ContainerType):
> > def __init__(self, name, members, attribute_list):
> > Type.__init__(self)
> > diff --git a/spice_codegen.py b/spice_codegen.py
> > index 4664740..66f99a5 100755
> > --- a/spice_codegen.py
> > +++ b/spice_codegen.py
> > @@ -177,6 +177,8 @@ parser.add_option("--license", dest="license",
> > help="license to use for generated file(s) (LGPL/BSD)",
> > default="LGPL")
> > parser.add_option("--generated-header", dest="generated_header",
> > metavar="FILE",
> > help="Name of the file to generate the header")
> > +parser.add_option("--generated-declaration-file",
> > dest="generated_declaration_file", metavar="FILE",
> > + help="Name of the file to generate declarations")
> >
> > (options, args) = parser.parse_args()
> >
> > @@ -259,6 +261,51 @@ else:
> > print >> sys.stderr, "Invalid license specified: %s" % options.license
> > sys.exit(1)
> >
> > +all_structures = {}
> > +def generate_declaration(t, writer_top):
> > + writer = codegen.CodeWriter()
> > + try:
> > + c_type = t.c_type()
> > + t.generate_c_declaration(writer)
> > + value = writer.getvalue().strip()
> > + if not value:
> > + return
> > + if c_type in all_structures:
> > + assert all_structures[c_type] == value, """Structure %s
> > redefinition
> > +previous:
> > +%s
> > +---
> > +current:
> > +%s
> > +---""" % (c_type, all_structures[c_type], value)
> > + else:
> > + all_structures[c_type] = value
> > + t.generate_c_declaration(writer_top)
> > + except:
> > + print >> sys.stderr, 'type %s' % t
> > + print >> sys.stderr, writer.getvalue()
> > + traceback.print_exc(sys.stderr)
> > +
> > +def generate_declarations():
> > + writer = codegen.CodeWriter()
> > + writer.public_prefix = options.prefix
> > + writer.write(license)
> > +
> > + # all types
> > + for t in ptypes.get_named_types():
> > + if isinstance(t, ptypes.StructType):
> > + generate_declaration(t, writer)
> > + if isinstance(t, ptypes.ChannelType):
> > + for m in t.client_messages + t.server_messages:
> > + generate_declaration(m.message_type, writer)
> > +
> > + content = writer.getvalue()
> > + write_content(options.generated_declaration_file, content,
> > + options.keep_identical_file)
> > +
> > +if options.generated_declaration_file:
> > + generate_declarations()
> > +
> > writer.public_prefix = options.prefix
> >
> > writer.writeln("/* this is a file autogenerated by spice_codegen.py */")
OT: I'm wondering why this code is so small while marshalling/demarshalling
is so long...
Frediano
More information about the Spice-devel
mailing list