[Spice-devel] [PATCH v6 18/42] dissector: Handle base fields
Frediano Ziglio
fziglio at redhat.com
Thu Aug 13 06:11:57 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>
---
python_modules/dissector.py | 67 ++++++++++++++++++++++++++++++++++
tests/Makefile.am | 36 +++++++++++++++----
tests/check_dissector | 50 ++++++++++++++++++++++++--
tests/data_base1 | Bin 0 -> 44 bytes
tests/data_empty | 0
tests/dissector_test.c | 22 ++++++++++--
tests/out_base1.txt | 85 ++++++++++++++++++++++++++++++++++++++++++++
tests/out_empty.txt | 1 +
tests/test.proto | 56 +++++++++++++++++++++++++++++
9 files changed, 305 insertions(+), 12 deletions(-)
create mode 100644 tests/data_base1
create mode 100644 tests/data_empty
create mode 100644 tests/out_base1.txt
create mode 100644 tests/out_empty.txt
create mode 100644 tests/test.proto
diff --git a/python_modules/dissector.py b/python_modules/dissector.py
index 777eb28..09048ba 100644
--- a/python_modules/dissector.py
+++ b/python_modules/dissector.py
@@ -191,6 +191,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", "%s.%s",' % (desc, dissector_short_name, 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
@@ -209,6 +275,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:
diff --git a/tests/Makefile.am b/tests/Makefile.am
index eaa0d59..0e8b214 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -7,7 +7,7 @@ AM_CPPFLAGS = \
$(GLIB2_CFLAGS) \
$(NULL)
-dissector_test_LDADD = \
+AM_LDFLAGS = \
$(GLIB2_LIBS) \
$(NULL)
@@ -22,22 +22,44 @@ 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_CPPFLAGS = $(AM_CPPFLAGS) -DDISSECTOR
+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/tests/check_dissector b/tests/check_dissector
index 7d6d086..bff3853 100755
--- a/tests/check_dissector
+++ b/tests/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/tests/data_base1 b/tests/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/tests/data_empty b/tests/data_empty
new file mode 100644
index 0000000..e69de29
diff --git a/tests/dissector_test.c b/tests/dissector_test.c
index ba87367..79f8592 100644
--- a/tests/dissector_test.c
+++ b/tests/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/tests/out_base1.txt b/tests/out_base1.txt
new file mode 100644
index 0000000..49a7426
--- /dev/null
+++ b/tests/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/tests/out_empty.txt b/tests/out_empty.txt
new file mode 100644
index 0000000..c00f815
--- /dev/null
+++ b/tests/out_empty.txt
@@ -0,0 +1 @@
+--- tree
diff --git a/tests/test.proto b/tests/test.proto
new file mode 100644
index 0000000..28d7769
--- /dev/null
+++ b/tests/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;
+};
--
2.4.3
More information about the Spice-devel
mailing list