Re: [Spice-devel] [PATCH spice-common 4/9] codegen: Allows to generate C declarations automatically
On Fri, Mar 08, 2019 at 04:47:35PM -0500, Frediano Ziglio wrote:
> >
> > On Sun, Mar 03, 2019 at 07:10:25PM +, 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
> > > ---
> > > 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.
Hmm, ok, might indeed be better to move them to a different commit.
Christophe
signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-common 4/9] codegen: Allows to generate C declarations automatically
>
> On Sun, Mar 03, 2019 at 07:10:25PM +, 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
> > ---
> > 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
>
> > +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
Re: [Spice-devel] [PATCH spice-common 4/9] codegen: Allows to generate C declarations automatically
On Sun, Mar 03, 2019 at 07:10:25PM +, 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
> ---
> 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?
Looks good otherwise,
Acked-by: Christophe Fergeau
> +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,
