[Spice-devel] [PATCH v3 27/51] Handle base fields
Christophe Fergeau
cfergeau at redhat.com
Thu Jul 23 06:17:53 PDT 2015
On Tue, Jul 21, 2015 at 05:45:57PM +0100, Frediano Ziglio wrote:
> 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.
>
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
> codegen/Makefile.am | 39 ++++++++++++++++----
> 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, 308 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
>
This commit introduces test.proto, and switch to using that in the tests
as far as I can tell, it would be nice to explain why the switch in the
commit log.
Christophe
> diff --git a/codegen/Makefile.am b/codegen/Makefile.am
> index b147b85..1281690 100644
> --- a/codegen/Makefile.am
> +++ b/codegen/Makefile.am
> @@ -6,7 +6,7 @@ AM_CPPFLAGS = \
> $(SPICE_COMMON_CFLAGS) \
> $(NULL)
>
> -dissector_test_LDADD = \
> +AM_LDFLAGS = \
> $(SPICE_COMMON_LIBS) \
> $(NULL)
>
> @@ -21,21 +21,46 @@ MARSHALLERS_DEPS = \
> $(top_srcdir)/spice_codegen.py \
> $(NULL)
>
> -test.o: test.h
> +test.o: test.h enums.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_test.o: test.h
> +
> +dissector.o: dissector.h packet-spice.h
> +
> +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
>
> 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 da89def..413aca4 100644
> --- a/python_modules/dissector.py
> +++ b/python_modules/dissector.py
> @@ -188,6 +188,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 encoding == 'ENC_LITTLE_ENDIAN':
> + 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
> @@ -206,6 +272,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
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20150723/0fe9b529/attachment-0001.sig>
More information about the Spice-devel
mailing list