[Spice-devel] [PATCH v4 17/41] dissector: Handle base fields

Frediano Ziglio fziglio at redhat.com
Thu Jul 23 08:54:34 PDT 2015


Add fields to base tree (so basically there is no tree).
Names is now generated from container + member name.
The check for duplicate is not that strong, should check if same field
is defined with same attributes.
This patch also introduce a test.proto file. This allows to do tests
which are not found in the spice.proto one. It allows more free changes
to spice.proto (for instance changing field names or a int32 to a flag32
don't change protocol at all but produce different dissector output).
Results of the test are easier to spot in a shorter file.

Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
---
 codegen/Makefile.am         |  35 ++++++++++++++----
 codegen/check_dissector     |  50 ++++++++++++++++++++++++--
 codegen/data_base1          | Bin 0 -> 44 bytes
 codegen/data_empty          |   0
 codegen/dissector_test.c    |  22 ++++++++++--
 codegen/out_base1.txt       |  85 ++++++++++++++++++++++++++++++++++++++++++++
 codegen/out_empty.txt       |   1 +
 codegen/test.proto          |  56 +++++++++++++++++++++++++++++
 python_modules/dissector.py |  67 ++++++++++++++++++++++++++++++++++
 9 files changed, 304 insertions(+), 12 deletions(-)
 create mode 100644 codegen/data_base1
 create mode 100644 codegen/data_empty
 create mode 100644 codegen/out_base1.txt
 create mode 100644 codegen/out_empty.txt
 create mode 100644 codegen/test.proto

diff --git a/codegen/Makefile.am b/codegen/Makefile.am
index f6b3a32..a95f40f 100644
--- a/codegen/Makefile.am
+++ b/codegen/Makefile.am
@@ -7,7 +7,7 @@ AM_CPPFLAGS =				\
 	$(SPICE_COMMON_CFLAGS)		\
 	$(NULL)
 
-dissector_test_LDADD =			\
+AM_LDFLAGS =				\
 	$(SPICE_COMMON_LIBS)		\
 	$(NULL)
 
@@ -22,22 +22,43 @@ MARSHALLERS_DEPS =					\
 	$(top_srcdir)/spice_codegen.py			\
 	$(NULL)
 
-BUILT_SOURCES = test.h
+BUILT_SOURCES = test.h enums.h dissector.h packet-spice.h
 
-test.c: $(top_srcdir)/spice.proto $(MARSHALLERS_DEPS)
-	$(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py --generate-dissector --client $< $@ >/dev/null
+test.c: test.proto $(MARSHALLERS_DEPS)
+	$(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py --generate-dissector $< --include epan/packet.h --include enums.h $@ >/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
+test.h: test.proto $(MARSHALLERS_DEPS)
+	$(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py --generate-dissector $< --header $@ >/dev/null
+
+enums.h: test.proto $(MARSHALLERS_DEPS)
+	$(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py --generate-wireshark-dissector $< $@ >/dev/null
+
+dissector.c: $(top_srcdir)/spice.proto $(MARSHALLERS_DEPS)
+	$(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py --generate-dissector $< $@ >/dev/null
+
+dissector.h: $(top_srcdir)/spice.proto $(MARSHALLERS_DEPS)
+	$(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py --generate-dissector $< --header $@ >/dev/null
+
+packet-spice.h: $(top_srcdir)/spice.proto $(MARSHALLERS_DEPS)
+	$(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py --generate-wireshark-dissector $< $@ >/dev/null
 
 TESTS = check_dissector
-check_PROGRAMS = dissector_test
+check_PROGRAMS = dissector_test compile_check
 
 dissector_test_SOURCES = dissector_test.c test.c test.h
+# this just to make sure the more complicate dissector continue to compile
+compile_check_SOURCES = dissector_test.c dissector.c dissector.h
 endif
 
 EXTRA_DIST =				\
 	check_dissector			\
+	test.proto			\
+	data_empty			\
+	out_empty.txt			\
+	data_base1			\
+	out_base1.txt			\
 	$(NULL)
 
+CLEANFILES = test.c test.h enums.h dissector.c dissector.h *.trs check_dissector.txt
+
 -include $(top_srcdir)/git.mk
diff --git a/codegen/check_dissector b/codegen/check_dissector
index 7d6d086..bff3853 100755
--- a/codegen/check_dissector
+++ b/codegen/check_dissector
@@ -1,13 +1,57 @@
 #!/bin/bash
-set -e
+
+# temporary file
+tmp=check_dissector.txt
 
 error() {
 	echo "$*" >&2
 	exit 1
 }
 
-./dissector_test 1 100 </dev/null
-if ./dissector_test 1 99 </dev/null; then
+set -ex
+
+test -x dissector_test
+
+# check some output
+# argument:
+# - input file
+# - channel
+# - message type
+# - output file to check
+# - rest copied
+check() {
+	local in channel msg_type out
+	set +x
+	in="$1"
+	shift
+	channel="$1"
+	shift
+	msg_type="$1"
+	shift
+	out="$1"
+	set +e
+	shift
+	./dissector_test -i "$in" "$@" $channel $msg_type > "$tmp"
+	res=$?
+	if [ $res -eq 0 ]; then
+		if [ "$out" != "" ]; then
+			diff -u $tmp "$out"
+			res=$?
+		fi
+	fi
+	set -ex
+	return $res
+}
+
+# check empty message
+check data_empty 1 100 out_empty.txt
+
+# check not existing message fails
+if check data_empty 1 99; then
 	error "This test should fail"
 fi
+
+# check just base types
+check data_base1 1 1 out_base1.txt
+
 exit 0
diff --git a/codegen/data_base1 b/codegen/data_base1
new file mode 100644
index 0000000000000000000000000000000000000000..853f3c845da12cba321d23b23507074ae7a6bd73
GIT binary patch
literal 44
lcmZo_WNc<^VPs-%1>!a!W?^MxZ->$yjEoG73=9lV3ILM91aSZW

literal 0
HcmV?d00001

diff --git a/codegen/data_empty b/codegen/data_empty
new file mode 100644
index 0000000..e69de29
diff --git a/codegen/dissector_test.c b/codegen/dissector_test.c
index ba87367..79f8592 100644
--- a/codegen/dissector_test.c
+++ b/codegen/dissector_test.c
@@ -192,6 +192,20 @@ static const char *describe_base(int base)
 	return NULL;
 }
 
+static bool
+lookup_values(char *label, value_string *vals, guint64 value)
+{
+	if (!vals)
+		return false;
+	for (;vals->strptr; ++vals) {
+		if (value == vals->value) {
+			sprintf(label, "%s (%" G_GINT64_MODIFIER "d)",vals->strptr, value);
+			return true;
+		}
+	}
+	return false;
+}
+
 WS_DLL_PUBLIC void
 proto_register_field_array(const int parent, hf_register_info *hf, const int num_records)
 {
@@ -328,7 +342,9 @@ proto_tree_add_item(proto_tree *tree, int hfindex, tvbuff_t *tvb,
 		size = 8;
 	str_uint:
 		uval = tvb_get_ule(tvb, start, size);
-		sprintf(label, "%" G_GINT64_MODIFIER "u", uval);
+		if (lookup_values(label, (value_string *) hfinfo->strings, uval))
+			break;
+		sprintf(label, "%" G_GINT64_MODIFIER "u (%#" G_GINT64_MODIFIER "x)", uval, uval);
 		break;
 	case FT_INT8:
 		size = 1;
@@ -343,7 +359,9 @@ proto_tree_add_item(proto_tree *tree, int hfindex, tvbuff_t *tvb,
 		size = 8;
 	str_int:
 		sval = tvb_get_sle(tvb, start, size);
-		sprintf(label, "%" G_GINT64_MODIFIER "d", sval);
+		if (lookup_values(label, (value_string *) hfinfo->strings, sval))
+			break;
+		sprintf(label, "%" G_GINT64_MODIFIER "d (%#" G_GINT64_MODIFIER "x)", sval, sval);
 		break;
 	case FT_BOOLEAN:
 		uval = tvb_get_ule(tvb, start, length);
diff --git a/codegen/out_base1.txt b/codegen/out_base1.txt
new file mode 100644
index 0000000..49a7426
--- /dev/null
+++ b/codegen/out_base1.txt
@@ -0,0 +1,85 @@
+--- tree
+    --- item
+    Text: 130 (0x82)
+    Name: u8
+    Abbrev: spice2.auto.msg_base_Base1_u8
+    Type: FT_UINT8
+    Base: BASE_DEC
+    --- item
+    Text: -127 (0xffffffffffffff81)
+    Name: i8
+    Abbrev: spice2.auto.msg_base_Base1_i8
+    Type: FT_INT8
+    Base: BASE_DEC
+    --- item
+    Text: 33537 (0x8301)
+    Name: u16
+    Abbrev: spice2.auto.msg_base_Base1_u16
+    Type: FT_UINT16
+    Base: BASE_DEC
+    --- item
+    Text: -31743 (0xffffffffffff8401)
+    Name: i16
+    Abbrev: spice2.auto.msg_base_Base1_i16
+    Type: FT_INT16
+    Base: BASE_DEC
+    --- item
+    Text: 2231566849 (0x85030201)
+    Name: u32
+    Abbrev: spice2.auto.msg_base_Base1_u32
+    Type: FT_UINT32
+    Base: BASE_DEC
+    --- item
+    Text: -2046623231 (0xffffffff86030201)
+    Name: i32
+    Abbrev: spice2.auto.msg_base_Base1_i32
+    Type: FT_INT32
+    Base: BASE_DEC
+    --- item
+    Text: 9729752138569155073 (0x8707060504030201)
+    Name: u64
+    Abbrev: spice2.auto.msg_base_Base1_u64
+    Type: FT_UINT64
+    Base: BASE_DEC
+    --- item
+    Text: -8644934341102468607 (0x8807060504030201)
+    Name: i64
+    Abbrev: spice2.auto.msg_base_Base1_i64
+    Type: FT_INT64
+    Base: BASE_DEC
+    --- item
+    Text: B (1)
+    Name: e8
+    Abbrev: spice2.auto.msg_base_Base1_e8
+    Type: FT_UINT8
+    Base: BASE_DEC
+    --- item
+    Text: 1 (0x1)
+    Name: e16
+    Abbrev: spice2.auto.msg_base_Base1_e16
+    Type: FT_UINT16
+    Base: BASE_DEC
+    --- item
+    Text: F (1)
+    Name: e32
+    Abbrev: spice2.auto.msg_base_Base1_e32
+    Type: FT_UINT32
+    Base: BASE_DEC
+    --- item
+    Text: H (1)
+    Name: f8
+    Abbrev: spice2.auto.msg_base_Base1_f8
+    Type: FT_UINT8
+    Base: BASE_HEX
+    --- item
+    Text: L (1)
+    Name: f16
+    Abbrev: spice2.auto.msg_base_Base1_f16
+    Type: FT_UINT16
+    Base: BASE_HEX
+    --- item
+    Text: N (1)
+    Name: f32
+    Abbrev: spice2.auto.msg_base_Base1_f32
+    Type: FT_UINT32
+    Base: BASE_HEX
diff --git a/codegen/out_empty.txt b/codegen/out_empty.txt
new file mode 100644
index 0000000..c00f815
--- /dev/null
+++ b/codegen/out_empty.txt
@@ -0,0 +1 @@
+--- tree
diff --git a/codegen/test.proto b/codegen/test.proto
new file mode 100644
index 0000000..28d7769
--- /dev/null
+++ b/codegen/test.proto
@@ -0,0 +1,56 @@
+/*
+This protocol file is meant to
+test some feature of the protocol
+*/
+
+message Empty {
+};
+
+enum8 E8 {
+	A, B, C
+};
+
+enum16 E16 {
+	D
+};
+
+enum32 E32 {
+	E, F, G
+};
+
+flags8 F8 {
+	H, I, J, K
+};
+
+flags16 F16 {
+	L, M
+};
+
+flags32 F32 {
+	N, O, P, Q
+};
+
+channel BaseChannel {
+  server:
+    message {
+        uint8 u8;
+        int8  i8;
+        uint16 u16;
+        int16  i16;
+        uint32 u32;
+        int32  i32;
+        uint64 u64;
+        int64  i64;
+	E8 e8;
+	E16 e16;
+	E32 e32;
+	F8 f8;
+	F16 f16;
+	F32 f32;
+    } Base1 = 1;
+    Empty empty = 100;
+};
+
+protocol Spice {
+    BaseChannel base = 1;
+};
diff --git a/python_modules/dissector.py b/python_modules/dissector.py
index 29b098a..5ba1fab 100644
--- a/python_modules/dissector.py
+++ b/python_modules/dissector.py
@@ -189,6 +189,72 @@ def primitive_read_func(t):
         return 'tvb_get_letoh64'
     raise NotImplementedError('primitive size not supported %s %s' % (size, t))
 
+def container_hf_name(container):
+    if container.has_name():
+        hf_name = "hf_%s" % container.name
+    else:
+        hf_name = "hf_%s" % container.c_name()
+    return hf_name
+
+def member_hf_name(container, member):
+    if member.member_type.is_array():
+        hf_name = "%s_array_%s" % (container_hf_name(container), member.name)
+    else:
+        hf_name = "%s_%s" % (container_hf_name(container), member.name)
+    return hf_name
+
+def get_primitive_ft_type(t):
+    assert(t.is_primitive())
+
+    unsigned = 'U'
+    if isinstance(t, ptypes.IntegerType) and t.signed:
+        unsigned = ''
+    size = t.get_fixed_nw_size()
+    assert size in (1, 2, 4, 8)
+    return "FT_%sINT%d" % (unsigned, size * 8)
+
+# write a field
+def write_wireshark_field(writer, container, member, t, tree, size, encoding='ENC_LITTLE_ENDIAN', prefix=''):
+
+    assert(member and container)
+
+    hf_name = member_hf_name(container, member)
+
+    # compute proper type
+    f_type = 'FT_NONE'
+    base = 'BASE_NONE'
+    vals = 'NULL'
+    if t.is_primitive():
+        assert(t.is_primitive())
+        base = 'BASE_DEC'
+        f_type = get_primitive_ft_type(t)
+        if isinstance(t, ptypes.FlagsType):
+            # show flag as hexadecimal for now
+            base = 'BASE_HEX'
+            assert(t.has_name())
+            vals = 'VALS(%s_vs)' % codegen.prefix_underscore_lower(t.name)
+        elif isinstance(t, ptypes.EnumType) or isinstance(t, ptypes.EnumBaseType):
+            base = 'BASE_DEC'
+            assert(t.has_name())
+            vals = 'VALS(%s_vs)' % codegen.prefix_underscore_lower(t.name)
+
+    desc = member.name
+    ws_name = 'auto.' + hf_name[3:]
+
+    writer.statement("%sproto_tree_add_item(%s, %s, glb->tvb, offset, %s, %s)" %
+            (prefix, tree, hf_name, size, encoding))
+
+    # TODO handle better duplications
+    if hf_writer.variable_defined(hf_name):
+        return
+    hf_writer.variable_def("static int", "%s = -1" % hf_name)
+
+    hf_defs.writeln('{ &%s,' % hf_name)
+    hf_defs.writeln('  { "%s", "spice2.%s",' % (desc, ws_name))
+    hf_defs.writeln('    %s, %s, %s, 0,' % (f_type, base, vals))
+    hf_defs.writeln('    NULL, HFILL }')
+    hf_defs.writeln('},')
+
 
 def write_switch(writer, container, switch, dest, scope):
     pass
@@ -207,6 +273,7 @@ def write_member_primitive(writer, container, member, t, dest, scope):
 
     if member.has_attr("bytes_count"):
         raise NotImplementedError("bytes_count not implemented")
+    write_wireshark_field(writer, container, member, t, dest.level.tree, t.get_fixed_nw_size())
     if member.has_attr("bytes_count"):
         dest_var = member.attributes["bytes_count"][0]
     else:
-- 
2.1.0



More information about the Spice-devel mailing list