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@xxxxxxxxxx> --- 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 _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel