[Spice-devel,v2,12/43] Generate some definition for dissector

Submitted by Frediano Ziglio on July 8, 2015, 1:53 p.m.

Details

Message ID 1436363656-4266-13-git-send-email-fziglio@redhat.com
State New
Headers show

Not browsing as part of any series.

Commit Message

Frediano Ziglio July 8, 2015, 1:53 p.m.
Generate global definitions.
Generate function to registers various dissector components.
For the moment the field array is empty bu we save some global to
be able to register new ones.
Add a base test for generated dissector.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
---
 Makefile.am                 |  2 +-
 codegen/Makefile.am         | 40 +++++++++++++++++++++
 codegen/dissector_test.c    | 81 +++++++++++++++++++++++++++++++++++++++++
 configure.ac                |  2 ++
 python_modules/dissector.py | 87 +++++++++++++++++++++++++++++++++++++++++++--
 spice_codegen.py            |  2 +-
 6 files changed, 209 insertions(+), 5 deletions(-)
 create mode 100644 codegen/Makefile.am
 create mode 100644 codegen/dissector_test.c

Patch hide | download patch | download mbox

diff --git a/Makefile.am b/Makefile.am
index 380bf24..382a0ea 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1,7 +1,7 @@ 
 NULL =
 ACLOCAL_AMFLAGS = -I m4
 
-SUBDIRS = python_modules common
+SUBDIRS = python_modules common codegen
 DIST_SUBDIRS = spice-protocol $(SUBDIRS)
 
 EXTRA_DIST =				\
diff --git a/codegen/Makefile.am b/codegen/Makefile.am
new file mode 100644
index 0000000..129543c
--- /dev/null
+++ b/codegen/Makefile.am
@@ -0,0 +1,40 @@ 
+NULL =
+
+AM_CPPFLAGS =				\
+	-I$(top_srcdir)			\
+	$(WIRESHARK_CFLAGS)			\
+	$(SPICE_COMMON_CFLAGS)		\
+	$(NULL)
+
+dissector_test_LDADD =				\
+	$(SPICE_COMMON_LIBS)				\
+	$(NULL)
+
+MARSHALLERS_DEPS =					\
+	$(top_srcdir)/python_modules/__init__.py	\
+	$(top_srcdir)/python_modules/codegen.py		\
+	$(top_srcdir)/python_modules/demarshal.py	\
+	$(top_srcdir)/python_modules/marshal.py		\
+	$(top_srcdir)/python_modules/ptypes.py		\
+	$(top_srcdir)/python_modules/spice_parser.py	\
+	$(top_srcdir)/python_modules/dissector.py	\
+	$(top_srcdir)/spice_codegen.py			\
+	$(NULL)
+
+test.o: test.h
+
+test.c: $(top_srcdir)/spice.proto $(MARSHALLERS_DEPS)
+	$(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py --generate-dissector --client $< $@ >/dev/null
+
+test.h: $(top_srcdir)/spice.proto $(MARSHALLERS_DEPS)
+	$(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py --generate-dissector --client $< --header $@ >/dev/null
+
+TESTS = dissector_test
+check_PROGRAMS = dissector_test
+
+dissector_test_SOURCES = dissector_test.c test.c test.h
+
+EXTRA_DIST =				\
+	$(NULL)
+
+-include $(top_srcdir)/git.mk
diff --git a/codegen/dissector_test.c b/codegen/dissector_test.c
new file mode 100644
index 0000000..6189da0
--- /dev/null
+++ b/codegen/dissector_test.c
@@ -0,0 +1,81 @@ 
+#undef NDEBUG
+#include <stdarg.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <getopt.h>
+#include <assert.h>
+
+#include <epan/expert.h>
+
+#include <test.h>
+
+enum {
+	first_hf_registered = 0x10000,
+	first_ei_registered = 0x20000,
+};
+static int last_hf_registered = first_hf_registered - 1;
+static int last_ei_registered = first_ei_registered - 1;
+
+WS_DLL_PUBLIC void
+proto_register_field_array(const int parent, hf_register_info *hf, const int num_records)
+{
+	int i;
+	assert(num_records >= 0);
+	for (i = 0; i < num_records; ++i) {
+		assert(hf);
+		assert(hf[i].p_id);
+		assert(*hf[i].p_id == -1);
+		*hf[i].p_id = ++last_hf_registered;
+	}
+}
+
+WS_DLL_PUBLIC void
+expert_register_field_array(expert_module_t* module, ei_register_info *ei, const int num_records)
+{
+	int i;
+	assert(num_records >= 0);
+	for (i = 0; i < num_records; ++i) {
+		assert(ei && ei[i].ids->ei == -1);
+		ei[i].ids->ei = ++last_ei_registered;
+	}
+}
+
+static const struct option long_options[] = {
+	{ "help", 0, NULL, 'h' },
+};
+static const char options[] = "h";
+
+static void syntax(FILE *f, int exit_value)
+{
+	fprintf(f,
+		"Usage: dissector_test [OPTIONS]\n"
+		"\n"
+		"Options:\n"
+		"  -h, --help               Show this help\n"
+	);
+	exit(exit_value);
+}
+
+int main(int argc, char **argv)
+{
+	while (1) {
+		int option_index;
+		int opt = getopt_long(argc, argv, options, long_options, &option_index);
+		if (opt == -1)
+			break;
+
+		switch (opt) {
+		case 'h':
+			syntax(stdout, EXIT_SUCCESS);
+			break;
+		default:
+			syntax(stderr, EXIT_FAILURE);
+			break;
+		}
+	}
+
+	spice_register_fields(1, NULL);
+
+	return EXIT_SUCCESS;
+}
diff --git a/configure.ac b/configure.ac
index 4287f92..a156cae 100644
--- a/configure.ac
+++ b/configure.ac
@@ -38,6 +38,7 @@  SPICE_CHECK_PIXMAN(SPICE_COMMON)
 SPICE_CHECK_SMARTCARD(SPICE_COMMON)
 SPICE_CHECK_CELT051(SPICE_COMMON)
 SPICE_CHECK_GLIB2(SPICE_COMMON)
+PKG_CHECK_MODULES(WIRESHARK, wireshark)
 SPICE_CHECK_OPUS(SPICE_COMMON)
 SPICE_CHECK_OPENGL(SPICE_COMMON)
 AC_SUBST(SPICE_COMMON_CFLAGS)
@@ -48,6 +49,7 @@  AC_CONFIG_FILES([
   Makefile
   common/Makefile
   python_modules/Makefile
+  codegen/Makefile
 ])
 
 AH_BOTTOM([
diff --git a/python_modules/dissector.py b/python_modules/dissector.py
index 90ba498..45f8737 100644
--- a/python_modules/dissector.py
+++ b/python_modules/dissector.py
@@ -1,14 +1,95 @@ 
 
 from . import codegen
+import re
+
+hf_writer = None
+hf_defs = None
+
+
+def write_parser_helpers(writer):
+    if writer.is_generated("helper", "demarshaller"):
+        return
+
+    writer.set_is_generated("helper", "demarshaller")
+
+    header = writer.header
+
+    header.writeln('#include <epan/packet.h>')
+    header.newline()
+
+    header.begin_block("typedef struct GlobalInfo")
+    header.variable_def("tvbuff_t *", "tvb")
+    header.variable_def("packet_info *", "pinfo")
+    header.variable_def("proto_item *", "msgtype_item")
+    header.variable_def("guint32", "message_offset")
+    header.variable_def("guint32", "message_end")
+    header.end_block(newline=False)
+    header.writeln(' GlobalInfo;')
+
+    header.newline()
+    header.statement('typedef guint32 (*spice_dissect_func_t)(guint16 message_type, GlobalInfo *glb, proto_tree *tree, guint32 offset)')
+
+    header.newline()
+    header.statement('void spice_register_fields(int proto, expert_module_t* expert_proto)')
+    header.statement('spice_dissect_func_t spice_client_channel_get_dissect(guint8 channel)')
+    header.statement('spice_dissect_func_t spice_server_channel_get_dissect(guint8 channel)')
+
+    writer = writer.function_helper()
+
+    writer.newline()
+    writer.statement('typedef guint32 (*parse_msg_func_t)(GlobalInfo *glb, proto_tree *tree, guint32 offset)')
+    writer.newline()
+    writer.statement('static expert_field ei_spice_unknown_message = EI_INIT')
+
+    writer.newline()
+    writer.writeln('#ifndef _U_')
+    writer.writeln('#define _U_')
+    writer.writeln('#endif')
+    writer.newline()
+
+
+def write_protocol_definitions(writer):
+    global hf_defs
+
+    writer.newline()
+    writer.function('spice_register_fields', 'void', 'int proto, expert_module_t* expert_proto')
+
+    writer.write("static hf_register_info hf[] = ")
+    writer.begin_block()
+    hf_defs = writer.get_subwriter()
+    writer.end_block(semicolon = True)
+    writer.newline()
+
+    writer.write('static ei_register_info ei[] =')
+    writer.begin_block()
+    writer.writeln('{ &ei_spice_unknown_message, { "spice.unknown_message", PI_UNDECODED, PI_WARN, "Unknown message - cannot dissect", EXPFILL }},')
+    writer.end_block(semicolon = True)
+    writer.newline()
+
+    writer.statement('proto_register_field_array(proto, hf, array_length(hf))')
+    writer.statement('expert_register_field_array(expert_proto, ei, array_length(ei))')
+    writer.end_block()
+
 
 def write_protocol_parser(writer, proto):
-    pass
+    global hf_writer
+
+    write_parser_helpers(writer)
+
+    # put fields declaration first
+    hf_writer = writer.get_subwriter()
+
+    # put fields definition at last
+    write_protocol_definitions(writer)
 
-def write_includes(writer):
+def write_includes(writer, dest_file):
+    header_name = re.sub(r'\.(c|cpp|cc|cxx|C)$', '.h', dest_file)
+    if header_name == dest_file:
+        header_name += '.h'
     writer.newline()
-    writer.writeln('#include <epan/packet.h>')
     writer.writeln('#include <epan/conversation.h>')
     writer.writeln('#include <epan/expert.h>')
     writer.newline()
+    writer.writeln('#include "%s"' % header_name)
     writer.writeln('#include "packet-spice.h"')
     writer.newline()
diff --git a/spice_codegen.py b/spice_codegen.py
index 8cfec1a..74d774f 100755
--- a/spice_codegen.py
+++ b/spice_codegen.py
@@ -224,7 +224,7 @@  if options.generate_dissector:
     if options.generate_demarshallers:
         print >> sys.stderr, "You can't specify --generate-demarshallers with --generate-dissector"
         sys.exit(1)
-    dissector.write_includes(writer)
+    dissector.write_includes(writer, dest_file)
     dissector.write_protocol_parser(writer, proto)
 
 if options.generate_demarshallers:

Comments

On Wed, Jul 08, 2015 at 02:53:45PM +0100, Frediano Ziglio wrote:
> Generate global definitions.
> Generate function to registers various dissector components.
> For the moment the field array is empty bu we save some global to
> be able to register new ones.
> Add a base test for generated dissector.
> 
> Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
> ---
>  Makefile.am                 |  2 +-
>  codegen/Makefile.am         | 40 +++++++++++++++++++++
>  codegen/dissector_test.c    | 81 +++++++++++++++++++++++++++++++++++++++++
>  configure.ac                |  2 ++
>  python_modules/dissector.py | 87 +++++++++++++++++++++++++++++++++++++++++++--
>  spice_codegen.py            |  2 +-
>  6 files changed, 209 insertions(+), 5 deletions(-)
>  create mode 100644 codegen/Makefile.am
>  create mode 100644 codegen/dissector_test.c
> 
> diff --git a/Makefile.am b/Makefile.am
> index 380bf24..382a0ea 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -1,7 +1,7 @@
>  NULL =
>  ACLOCAL_AMFLAGS = -I m4
>  
> -SUBDIRS = python_modules common
> +SUBDIRS = python_modules common codegen
>  DIST_SUBDIRS = spice-protocol $(SUBDIRS)
>  
>  EXTRA_DIST =				\
> diff --git a/codegen/Makefile.am b/codegen/Makefile.am
> new file mode 100644
> index 0000000..129543c
> --- /dev/null
> +++ b/codegen/Makefile.am
> @@ -0,0 +1,40 @@
> +NULL =
> +
> +AM_CPPFLAGS =				\
> +	-I$(top_srcdir)			\
> +	$(WIRESHARK_CFLAGS)			\
> +	$(SPICE_COMMON_CFLAGS)		\
> +	$(NULL)
> +
> +dissector_test_LDADD =				\
> +	$(SPICE_COMMON_LIBS)				\
> +	$(NULL)
> +
> +MARSHALLERS_DEPS =					\
> +	$(top_srcdir)/python_modules/__init__.py	\
> +	$(top_srcdir)/python_modules/codegen.py		\
> +	$(top_srcdir)/python_modules/demarshal.py	\
> +	$(top_srcdir)/python_modules/marshal.py		\
> +	$(top_srcdir)/python_modules/ptypes.py		\
> +	$(top_srcdir)/python_modules/spice_parser.py	\
> +	$(top_srcdir)/python_modules/dissector.py	\
> +	$(top_srcdir)/spice_codegen.py			\
> +	$(NULL)
> +
> +test.o: test.h

This test.o dep (and the similar one in another commit) is odd. Missing
BUILT_SOURCES?

> +
> +test.c: $(top_srcdir)/spice.proto $(MARSHALLERS_DEPS)
> +	$(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py --generate-dissector --client $< $@ >/dev/null
> +
> +test.h: $(top_srcdir)/spice.proto $(MARSHALLERS_DEPS)
> +	$(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py --generate-dissector --client $< --header $@ >/dev/null
> +
> +TESTS = dissector_test
> +check_PROGRAMS = dissector_test
> +
> +dissector_test_SOURCES = dissector_test.c test.c test.h
> +
> +EXTRA_DIST =				\
> +	$(NULL)
> +
> +-include $(top_srcdir)/git.mk


> diff --git a/configure.ac b/configure.ac
> index 4287f92..a156cae 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -38,6 +38,7 @@ SPICE_CHECK_PIXMAN(SPICE_COMMON)
>  SPICE_CHECK_SMARTCARD(SPICE_COMMON)
>  SPICE_CHECK_CELT051(SPICE_COMMON)
>  SPICE_CHECK_GLIB2(SPICE_COMMON)
> +PKG_CHECK_MODULES(WIRESHARK, wireshark)

This should be optional.

Christophe
----- Original Message -----
> From: "Christophe Fergeau" <cfergeau@redhat.com>
> To: "Frediano Ziglio" <fziglio@redhat.com>
> Cc: spice-devel@lists.freedesktop.org
> Sent: Tuesday, July 21, 2015 4:37:15 PM
> Subject: Re: [Spice-devel] [PATCH v2 12/43] Generate some definition for dissector
> 
> On Wed, Jul 08, 2015 at 02:53:45PM +0100, Frediano Ziglio wrote:
> > Generate global definitions.
> > Generate function to registers various dissector components.
> > For the moment the field array is empty bu we save some global to
> > be able to register new ones.
> > Add a base test for generated dissector.
> > 
> > Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
> > ---
> >  Makefile.am                 |  2 +-
> >  codegen/Makefile.am         | 40 +++++++++++++++++++++
> >  codegen/dissector_test.c    | 81 +++++++++++++++++++++++++++++++++++++++++
> >  configure.ac                |  2 ++
> >  python_modules/dissector.py | 87
> >  +++++++++++++++++++++++++++++++++++++++++++--
> >  spice_codegen.py            |  2 +-
> >  6 files changed, 209 insertions(+), 5 deletions(-)
> >  create mode 100644 codegen/Makefile.am
> >  create mode 100644 codegen/dissector_test.c
> > 
> > diff --git a/Makefile.am b/Makefile.am
> > index 380bf24..382a0ea 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -1,7 +1,7 @@
> >  NULL =
> >  ACLOCAL_AMFLAGS = -I m4
> >  
> > -SUBDIRS = python_modules common
> > +SUBDIRS = python_modules common codegen
> >  DIST_SUBDIRS = spice-protocol $(SUBDIRS)
> >  
> >  EXTRA_DIST =				\
> > diff --git a/codegen/Makefile.am b/codegen/Makefile.am
> > new file mode 100644
> > index 0000000..129543c
> > --- /dev/null
> > +++ b/codegen/Makefile.am
> > @@ -0,0 +1,40 @@
> > +NULL =
> > +
> > +AM_CPPFLAGS =				\
> > +	-I$(top_srcdir)			\
> > +	$(WIRESHARK_CFLAGS)			\
> > +	$(SPICE_COMMON_CFLAGS)		\
> > +	$(NULL)
> > +
> > +dissector_test_LDADD =				\
> > +	$(SPICE_COMMON_LIBS)				\
> > +	$(NULL)
> > +
> > +MARSHALLERS_DEPS =					\
> > +	$(top_srcdir)/python_modules/__init__.py	\
> > +	$(top_srcdir)/python_modules/codegen.py		\
> > +	$(top_srcdir)/python_modules/demarshal.py	\
> > +	$(top_srcdir)/python_modules/marshal.py		\
> > +	$(top_srcdir)/python_modules/ptypes.py		\
> > +	$(top_srcdir)/python_modules/spice_parser.py	\
> > +	$(top_srcdir)/python_modules/dissector.py	\
> > +	$(top_srcdir)/spice_codegen.py			\
> > +	$(NULL)
> > +
> > +test.o: test.h
> 
> This test.o dep (and the similar one in another commit) is odd. Missing
> BUILT_SOURCES?
> 

No, it's just manual, see http://www.delorie.com/gnu/docs/automake/automake_69.html and http://www.delorie.com/gnu/docs/automake/automake_70.html, basically not all cases are covered by automatic generated dependency.

> > +
> > +test.c: $(top_srcdir)/spice.proto $(MARSHALLERS_DEPS)
> > +	$(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py --generate-dissector
> > --client $< $@ >/dev/null
> > +
> > +test.h: $(top_srcdir)/spice.proto $(MARSHALLERS_DEPS)
> > +	$(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py --generate-dissector
> > --client $< --header $@ >/dev/null
> > +
> > +TESTS = dissector_test
> > +check_PROGRAMS = dissector_test
> > +
> > +dissector_test_SOURCES = dissector_test.c test.c test.h
> > +
> > +EXTRA_DIST =				\
> > +	$(NULL)
> > +
> > +-include $(top_srcdir)/git.mk
> 
> 
> > diff --git a/configure.ac b/configure.ac
> > index 4287f92..a156cae 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -38,6 +38,7 @@ SPICE_CHECK_PIXMAN(SPICE_COMMON)
> >  SPICE_CHECK_SMARTCARD(SPICE_COMMON)
> >  SPICE_CHECK_CELT051(SPICE_COMMON)
> >  SPICE_CHECK_GLIB2(SPICE_COMMON)
> > +PKG_CHECK_MODULES(WIRESHARK, wireshark)
> 
> This should be optional.
> 

Oh... is it not in this way? I though so.

> Christophe
> 

Frediano
On Tue, Jul 21, 2015 at 11:54:06AM -0400, Frediano Ziglio wrote:
> > From: "Christophe Fergeau" <cfergeau@redhat.com>
> > This test.o dep (and the similar one in another commit) is odd. Missing
> > BUILT_SOURCES?
> > 
> 
> No, it's just manual, see
> http://www.delorie.com/gnu/docs/automake/automake_69.html and
> http://www.delorie.com/gnu/docs/automake/automake_70.html, basically
> not all cases are covered by automatic generated dependency.

Stuff like "Adding explicit dependencies like this can be a bit
dangerous if you are not careful enough." "Always check the generated
`Makefile.in' if you do this." would make me strongly favour use of
BUILT_SOURCES, especially as "make" in the codegen directory will only
generate the needed sources (make check will then build the code).

> 
> > > +
> > > +test.c: $(top_srcdir)/spice.proto $(MARSHALLERS_DEPS)
> > > +	$(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py --generate-dissector
> > > --client $< $@ >/dev/null
> > > +
> > > +test.h: $(top_srcdir)/spice.proto $(MARSHALLERS_DEPS)
> > > +	$(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py --generate-dissector
> > > --client $< --header $@ >/dev/null
> > > +
> > > +TESTS = dissector_test
> > > +check_PROGRAMS = dissector_test
> > > +
> > > +dissector_test_SOURCES = dissector_test.c test.c test.h
> > > +
> > > +EXTRA_DIST =				\
> > > +	$(NULL)
> > > +
> > > +-include $(top_srcdir)/git.mk
> > 
> > 
> > > diff --git a/configure.ac b/configure.ac
> > > index 4287f92..a156cae 100644
> > > --- a/configure.ac
> > > +++ b/configure.ac
> > > @@ -38,6 +38,7 @@ SPICE_CHECK_PIXMAN(SPICE_COMMON)
> > >  SPICE_CHECK_SMARTCARD(SPICE_COMMON)
> > >  SPICE_CHECK_CELT051(SPICE_COMMON)
> > >  SPICE_CHECK_GLIB2(SPICE_COMMON)
> > > +PKG_CHECK_MODULES(WIRESHARK, wireshark)
> > 
> > This should be optional.
> > 
> 
> Oh... is it not in this way? I though so.

Nope, this errors out if wireshark is not found (easy to test by
changing 'wireshark' to something random). If you want a
--enable-dissector argument to configure, you can take a look at
m4/spice-deps.m4 for an example of how to handle it (defaults to
auto-detection, errors out if --enable-dissector is passed but wireshark
is not found, always disables it if --disable-dissector is passed).
Here, I think just doing
PKG_CHECK_MODULES(WIRESHARK, [have_wireshark=yes], [have_wireshark=no])
AM_CONDITIONAL([HAVE_WIRESHARK], [test "x$have_wireshark" = "xyes"])
should be enough (and then add the appropriate ifdef to the Makefile.am)

Christophe