Re: [PATCH v3 27/51] Handle base fields

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

Attachment: pgppRrimfQ1bm.pgp
Description: PGP signature

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel

[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]