[Spice-devel] [spice-common 3/3] codegen: Autogenerate client_marshallers.h

Frediano Ziglio fziglio at redhat.com
Tue Jun 21 11:22:34 UTC 2016


> 
> On Tue, Jun 21, 2016 at 05:15:54AM -0400, Frediano Ziglio wrote:
> > About the macro, what about
> > 
> > 
> > diff --git a/python_modules/marshal.py b/python_modules/marshal.py
> > index cd4b4fa..c2235e2 100644
> > --- a/python_modules/marshal.py
> > +++ b/python_modules/marshal.py
> > @@ -1,13 +1,20 @@
> >  
> >  from . import ptypes
> >  from . import codegen
> > +import re
> >  
> >  def write_includes(writer):
> >      writer.header.writeln("#include <spice/protocol.h>")
> >      writer.header.writeln('#include "common/marshaller.h"')
> >      writer.header.newline()
> > -    writer.header.writeln("#ifndef _GENERATED_HEADERS_H")
> > -    writer.header.writeln("#define _GENERATED_HEADERS_H")
> > +    if writer.header.has_option("dest_file"):
> > +        src = writer.header.options["dest_file"]
> > +    else:
> > +        src = "generated_headers.h"
> > +    src = re.sub(r'[^a-z0-9]+', '_', 'file_' + src, flags=re.IGNORECASE)
> > +    src = src.upper()
> > +    writer.header.writeln("#ifndef %s" % src)
> > +    writer.header.writeln("#define %s" % src)
> 
> Looks good to me, I'd probably squash this in:
> 
> --- a/python_modules/marshal.py
> +++ b/python_modules/marshal.py
> @@ -11,8 +11,10 @@ def write_includes(writer):
>          src = writer.header.options["dest_file"]
>      else:
>          src = "generated_headers.h"
> -    src = re.sub(r'[^a-z0-9]+', '_', 'file_' + src, flags=re.IGNORECASE)
> +    src = re.sub(r'[^a-z0-9]+', '_', src, flags=re.IGNORECASE)
>      src = src.upper()
> +    if src.endswith("_H"):
> +        src = "_H_"+src[:-2]

This is not C99 compatible

>      writer.header.writeln("#ifndef %s" % src)
>      writer.header.writeln("#define %s" % src)
> 
> 
> >  
> >      writer.writeln("#include <string.h>")
> >      writer.writeln("#include <assert.h>")
> > diff --git a/spice_codegen.py b/spice_codegen.py
> > index 569cccc..53c7be3 100755
> > --- a/spice_codegen.py
> > +++ b/spice_codegen.py
> > @@ -170,6 +170,7 @@ if proto == None:
> >  codegen.set_prefix(proto.name)
> >  writer = codegen.CodeWriter()
> >  writer.header = codegen.CodeWriter()
> > +writer.header.set_option("dest_file", dest_file)
> >  writer.set_option("source", os.path.basename(proto_file))
> >  
> >  license = """/*
> > 
> > 
> > Frediano
> > 
> > > 
> > > On Mon, Jun 20, 2016 at 05:06:25PM +0200, Christophe Fergeau wrote:
> > > > This commit adds autogeneration of a generated_client_marshallers.h
> > > > header, which is then included in client_marshallers.h
> > > > 
> > > > This allows to remove the SpiceMessageMarshallers struct from this
> > > > file,
> > > > which has to match what the generated code expects.
> > > > ---
> > > >  common/Makefile.am          |  6 +++++-
> > > >  common/client_marshallers.h | 46
> > > >  +--------------------------------------------
> > > >  python_modules/marshal.py   | 27 +++++++++++++++++++++++++-
> > > >  3 files changed, 32 insertions(+), 47 deletions(-)
> > > > 
> > > > diff --git a/common/Makefile.am b/common/Makefile.am
> > > > index 2dd56f3..4b8f63a 100644
> > > > --- a/common/Makefile.am
> > > > +++ b/common/Makefile.am
> > > > @@ -5,6 +5,7 @@ CLIENT_MARSHALLERS =				\
> > > >  	generated_client_demarshallers.c	\
> > > >  	generated_client_demarshallers1.c	\
> > > >  	generated_client_marshallers.c		\
> > > > +	generated_client_marshallers.h		\
> > > >  	generated_client_marshallers1.c		\
> > > >  	$(NULL)
> > > >  
> > > > @@ -107,8 +108,11 @@ generated_client_demarshallers.c:
> > > > $(top_srcdir)/spice.proto $(MARSHALLERS_DEPS)
> > > >  generated_client_demarshallers1.c: $(top_srcdir)/spice1.proto
> > > >  $(MARSHALLERS_DEPS)
> > > >  	$(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py
> > > >  	--generate-demarshallers --client --include common/messages.h
> > > >  	--prefix 1
> > > >  	--ptrsize 8 $< $@ >/dev/null
> > > >  
> > > > +generated_client_marshallers.h: $(top_srcdir)/spice.proto
> > > > $(MARSHALLERS_DEPS)
> > > > +	$(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py
> > > > --generate-marshallers -P --client --include common/messages.h -H $< $@
> > > > >/dev/null
> > > > +
> > > >  generated_client_marshallers.c: $(top_srcdir)/spice.proto
> > > >  $(MARSHALLERS_DEPS)
> > > > -	$(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py
> > > > --generate-marshallers -P --include common/messages.h --include
> > > > client_marshallers.h --client $< $@ >/dev/null
> > > > +	$(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py
> > > > --generate-marshallers -P --include client_marshallers.h --client $< $@
> > > > >/dev/null
> > > >  
> > > >  generated_client_marshallers1.c: $(top_srcdir)/spice1.proto
> > > >  $(MARSHALLERS_DEPS)
> > > >  	$(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py
> > > >  	--generate-marshallers -P --include common/messages.h --include
> > > >  	client_marshallers.h --client --prefix 1 --ptrsize 8 $< $@ >/dev/null
> > > > diff --git a/common/client_marshallers.h b/common/client_marshallers.h
> > > > index 2074323..d3e26b4 100644
> > > > --- a/common/client_marshallers.h
> > > > +++ b/common/client_marshallers.h
> > > > @@ -25,56 +25,12 @@
> > > >  
> > > >  #include <spice/protocol.h>
> > > >  
> > > > +#include "generated_client_marshallers.h"
> > > >  #include "marshaller.h"
> > > >  #include "messages.h"
> > > >  
> > > >  SPICE_BEGIN_DECLS
> > > >  
> > > > -typedef struct {
> > > > -    void (*msg_SpiceMsgEmpty)(SpiceMarshaller *m, SpiceMsgEmpty *msg);
> > > > -    void (*msg_SpiceMsgData)(SpiceMarshaller *m, SpiceMsgData *msg);
> > > > -    void (*msg_SpiceMsgCompressedData)(SpiceMarshaller *m,
> > > > SpiceMsgCompressedData *msg);
> > > > -    void (*msgc_ack_sync)(SpiceMarshaller *m, SpiceMsgcAckSync *msg);
> > > > -    void (*msgc_pong)(SpiceMarshaller *m, SpiceMsgPing *msg);
> > > > -    void (*msgc_disconnecting)(SpiceMarshaller *m, SpiceMsgDisconnect
> > > > *msg);
> > > > -    void (*msgc_main_client_info)(SpiceMarshaller *m,
> > > > SpiceMsgcClientInfo
> > > > *msg);
> > > > -    void (*msgc_main_mouse_mode_request)(SpiceMarshaller *m,
> > > > SpiceMsgcMainMouseModeRequest *msg);
> > > > -    void (*msgc_main_agent_start)(SpiceMarshaller *m,
> > > > SpiceMsgcMainAgentStart *msg);
> > > > -    void (*msgc_main_agent_token)(SpiceMarshaller *m,
> > > > SpiceMsgcMainAgentTokens *msg);
> > > > -    void (*msgc_main_migrate_dst_do_seamless)(SpiceMarshaller *m,
> > > > SpiceMsgcMainMigrateDstDoSeamless *msg);
> > > > -    void (*msgc_display_init)(SpiceMarshaller *m, SpiceMsgcDisplayInit
> > > > *msg);
> > > > -    void (*msgc_display_stream_report)(SpiceMarshaller *m,
> > > > SpiceMsgcDisplayStreamReport *msg);
> > > > -    void (*msgc_display_gl_draw_done)(SpiceMarshaller *m,
> > > > SpiceMsgcDisplayGlDrawDone *msg);
> > > > -    void (*msgc_inputs_key_down)(SpiceMarshaller *m, SpiceMsgcKeyDown
> > > > *msg);
> > > > -    void (*msgc_inputs_key_up)(SpiceMarshaller *m, SpiceMsgcKeyUp
> > > > *msg);
> > > > -    void (*msgc_inputs_key_modifiers)(SpiceMarshaller *m,
> > > > SpiceMsgcKeyModifiers *msg);
> > > > -    void (*msgc_inputs_mouse_motion)(SpiceMarshaller *m,
> > > > SpiceMsgcMouseMotion *msg);
> > > > -    void (*msgc_inputs_mouse_position)(SpiceMarshaller *m,
> > > > SpiceMsgcMousePosition *msg);
> > > > -    void (*msgc_inputs_mouse_press)(SpiceMarshaller *m,
> > > > SpiceMsgcMousePress *msg);
> > > > -    void (*msgc_inputs_mouse_release)(SpiceMarshaller *m,
> > > > SpiceMsgcMouseRelease *msg);
> > > > -    void (*msgc_record_data)(SpiceMarshaller *m, SpiceMsgcRecordPacket
> > > > *msg);
> > > > -    void (*msgc_record_mode)(SpiceMarshaller *m, SpiceMsgcRecordMode
> > > > *msg);
> > > > -    void (*msgc_record_start_mark)(SpiceMarshaller *m,
> > > > SpiceMsgcRecordStartMark *msg);
> > > > -    void (*msgc_tunnel_service_add)(SpiceMarshaller *m,
> > > > SpiceMsgcTunnelAddGenericService *msg, SpiceMarshaller **name_out,
> > > > SpiceMarshaller **description_out);
> > > > -    void (*msgc_tunnel_service_remove)(SpiceMarshaller *m,
> > > > SpiceMsgcTunnelRemoveService *msg);
> > > > -    void (*msgc_tunnel_socket_open_ack)(SpiceMarshaller *m,
> > > > SpiceMsgcTunnelSocketOpenAck *msg);
> > > > -    void (*msgc_tunnel_socket_open_nack)(SpiceMarshaller *m,
> > > > SpiceMsgcTunnelSocketOpenNack *msg);
> > > > -    void (*msgc_tunnel_socket_fin)(SpiceMarshaller *m,
> > > > SpiceMsgcTunnelSocketFin *msg);
> > > > -    void (*msgc_tunnel_socket_closed)(SpiceMarshaller *m,
> > > > SpiceMsgcTunnelSocketClosed *msg);
> > > > -    void (*msgc_tunnel_socket_closed_ack)(SpiceMarshaller *m,
> > > > SpiceMsgcTunnelSocketClosedAck *msg);
> > > > -    void (*msgc_tunnel_socket_data)(SpiceMarshaller *m,
> > > > SpiceMsgcTunnelSocketData *msg);
> > > > -    void (*msgc_tunnel_socket_token)(SpiceMarshaller *m,
> > > > SpiceMsgcTunnelSocketTokens *msg);
> > > > -#ifdef USE_SMARTCARD
> > > > -    void (*msgc_smartcard_atr)(SpiceMarshaller *m, VSCMsgATR *msg);
> > > > -    void (*msgc_smartcard_error)(SpiceMarshaller *m, VSCMsgError
> > > > *msg);
> > > > -    void (*msgc_smartcard_header)(SpiceMarshaller *m, VSCMsgHeader
> > > > *msg);
> > > > -    void (*msgc_smartcard_data)(SpiceMarshaller *m, SpiceMsgcSmartcard
> > > > *msg, SpiceMarshaller **reader_name_out);
> > > > -    void (*msgc_smartcard_reader_add)(SpiceMarshaller *m,
> > > > VSCMsgReaderAdd
> > > > *msg);
> > > > -#endif
> > > > -    void (*msgc_port_event)(SpiceMarshaller *m, SpiceMsgcPortEvent
> > > > *msg);
> > > > -    void (*msgc_display_preferred_compression)(SpiceMarshaller *m,
> > > > SpiceMsgcDisplayPreferredCompression *msg);
> > > > -} SpiceMessageMarshallers;
> > > > -
> > > >  SpiceMessageMarshallers *spice_message_marshallers_get(void);
> > > >  SpiceMessageMarshallers *spice_message_marshallers_get1(void);
> > > >  
> > > > diff --git a/python_modules/marshal.py b/python_modules/marshal.py
> > > > index cc6cbdf..cd4b4fa 100644
> > > > --- a/python_modules/marshal.py
> > > > +++ b/python_modules/marshal.py
> > > > @@ -336,6 +336,7 @@ def write_container_marshaller(writer, container,
> > > > src):
> > > >  def write_message_marshaller(writer, message, private):
> > > >      if message.has_attr("ifdef"):
> > > >          writer.ifdef(message.attributes["ifdef"][0])
> > > > +        writer.header.ifdef(message.attributes["ifdef"][0])
> > > >      writer.out_prefix = ""
> > > >      function_name = "spice_marshall_" + message.c_name()
> > > >      if writer.is_generated("marshaller", function_name):
> > > > @@ -348,7 +349,15 @@ def write_message_marshaller(writer, message,
> > > > private):
> > > >          n = [", SpiceMarshaller **%s_out" % name for name in names]
> > > >          names_args = "".join(n)
> > > >  
> > > > -    if not private:
> > > > +    if private:
> > > > +        message_name = message.c_name()
> > > > +        if (not message_name.startswith("msgc_")):
> > > > +            #small bug above, checks for startswith("msg") which
> > > > +            #matches "msgc" and appends "msg_" if this fails causing
> > > > +            #inconsistencies
> > > > +            message_name = "msg_" + message_name
> > > > +        writer.header.writeln("void (*" + message_name +
> > > > ")(SpiceMarshaller *m, %s *msg" % message.c_type() + names_args + ");")
> > > > +    else:
> > > >          writer.header.writeln("void " + function_name +
> > > >          "(SpiceMarshaller
> > > >          *m, %s *msg" % message.c_type() + names_args + ");")
> > > >  
> > > >      scope = writer.function(function_name,
> > > > @@ -369,11 +378,18 @@ def write_message_marshaller(writer, message,
> > > > private):
> > > >      writer.end_block()
> > > >      if message.has_attr("ifdef"):
> > > >          writer.endif(message.attributes["ifdef"][0])
> > > > +        writer.header.endif(message.attributes["ifdef"][0])
> > > > +
> > > >      writer.newline()
> > > >      return function_name
> > > >  
> > > >  def write_protocol_marshaller(writer, proto, is_server,
> > > >  private_marshallers):
> > > >      functions = {}
> > > > +    writer.header.newline()
> > > > +    writer.header.writeln("SPICE_BEGIN_DECLS")
> > > > +    writer.header.newline()
> > > 
> > > This is better in write_includes() imo
> > > 
> > > > @@ -416,5 +438,8 @@ def write_protocol_marshaller(writer, proto,
> > > > is_server,
> > > > private_marshallers):
> > > >          writer.end_block()
> > > >          writer.newline()
> > > >  
> > > > +    writer.header.writeln("SPICE_END_DECLS")
> > > > +    writer.header.newline()
> > > > +
> > > 
> > > and this in write_trailer()
> > > 
> > > otherwise we get an odd generated_server_marshallers.h file with
> > > declaration after SPICE_END_DECLS.
> > > Squashing this in:
> > > 
> > > diff --git a/python_modules/marshal.py b/python_modules/marshal.py
> > > index cd4b4fa..6597edf 100644
> > > --- a/python_modules/marshal.py
> > > +++ b/python_modules/marshal.py
> > > @@ -8,6 +8,9 @@ def write_includes(writer):
> > >      writer.header.newline()
> > >      writer.header.writeln("#ifndef _GENERATED_HEADERS_H")
> > >      writer.header.writeln("#define _GENERATED_HEADERS_H")
> > > +    writer.header.newline()
> > > +    writer.header.writeln("SPICE_BEGIN_DECLS")
> > > +    writer.header.newline()
> > > 
> > >      writer.writeln("#include <string.h>")
> > >      writer.writeln("#include <assert.h>")
> > > @@ -385,9 +388,6 @@ def write_message_marshaller(writer, message,
> > > private):
> > > 
> > >  def write_protocol_marshaller(writer, proto, is_server,
> > >  private_marshallers):
> > >      functions = {}
> > > -    writer.header.newline()
> > > -    writer.header.writeln("SPICE_BEGIN_DECLS")
> > > -    writer.header.newline()
> > >      if private_marshallers:
> > >          writer.header.begin_block("typedef struct")
> > >      for c in proto.channels:
> > > @@ -438,8 +438,9 @@ def write_protocol_marshaller(writer, proto,
> > > is_server,
> > > private_marshallers):
> > >          writer.end_block()
> > >          writer.newline()
> > > 
> > > -    writer.header.writeln("SPICE_END_DECLS")
> > > -    writer.header.newline()
> > > -
> > >  def write_trailer(writer):
> > > +    writer.header.newline()
> > > +    writer.header.writeln("SPICE_END_DECLS")
> > > +    writer.header.newline()
> > > +
> > >      writer.header.writeln("#endif")
> > > 
> > > 
> > > 
> > > Christophe
> > > 
> > > 
> > > >  def write_trailer(writer):
> > > >      writer.header.writeln("#endif")



More information about the Spice-devel mailing list