Wireshark-dev: Re: [Wireshark-dev] [RFC] bug 6797 : proto: Add proto_tree_add_split_{uint, int}
On 07/02/2012 07:06, Sylvain Munaut wrote:
Hi,
Anders Broman suggested I post here to get more feedback.
----
This adds a helper in proto.c to display fields where you have to go grab bits
at several places (within a 32 bits word) to make the value.
This doesn't recover the value of the field itself, it just takes care of the
formating and displaying of 'bit fields' in the common format to display such
things.
(thing like ....110...1100...1 = 0xd9)
This will be used in the GMR-1 RR dissector among others where some fields have
been split into small values (i.e. MSB at one place, LSB at another).
If you don't think this belongs in proto and should be somewhere else, I can
prefix it with gmr1_ and place it in gmr1_common.c (it does need to be exported
in the global namespace tough because it's used in several gmr1 files).
----
And the feedback already in the bug tracker:
We would need a write up of the function in README.developer as well.
Ok, will do.
You should probably post to the developers list to get more feedback.
Done :)
Would it be feasible to have proto_tree_add_split_item() instead/as well
and have all the magic done internally?
There is two issues there:
- First the caller code can often decode the real value much more
easily than having to reconstruct it bit by bit.
- Some time to get the value you have to take the bits in a weird
"order". Like if you get something like
...3210...7654 (dots are unused bits, numbers are bit position).
The caller knows he can just do (val& 0xf)<< 4 | ((val& 0xf00)>> 8)
But passing the info on which bit to take when is not trivial.
Here I'm aiming to provide visual feedback about which bits where used
rather than how they were used.
[snip]
I recently found a need for such a function for the gsm_rlcmac dissector
for EGPRS header fields which have split uint encoding.
I have just written the function:
/** Add bits to a proto_tree, using the text label registered to that item.
* The item is extracted from the tvbuff handed to it as a set
* of segments of contiguous bits, specified by an array of
* bits_spec elements. The segments are assembled (in arbitrary
* order) to create the value. There may be any number of
* segments specifying up to a total of 64 bits which may occur
* anywhere within the tvb.
@param tree the tree to append this item to
@param hf_index field index. Fields for use with this function should
have bitmask==0.
@param tvb the tv buffer of the current data
@param bit_offset start of data in tvb expressed in bits
@param no_of_bits length of data in tvb expressed in bits
@param return_value if a pointer is passed here the value is returned.
@param little_endian big or little endian byte representation
@return the newly created item */
extern proto_item *
proto_tree_add_split_bits_ret_val(proto_tree *tree, const int hf_index,
tvbuff_t *tvb,
const gint bit_offset, const bits_spec_t *bits_spec,
guint64 *return_value, const gboolean little_endian);
which is configured with an array of segment descriptors that allow the
size, position and packing order of segments to be specified.
/*
* This structure describes one segment of a split_bits element
* seg_offset is the bit offset in the input stream of the first (most
significant) bit of this segment
* seg_length is the number of contiguous bits of this segment.
* the first element of an array of bits_specs describes the most
significant segment of the output value.
* Segments should not overlap (i.e. any input bit should not be
contained in more than one segment).
* Following the last bits_spec is a sentinal entry with seg_length of
0 which denotes the end of the array.
*/
typedef struct
{
guint seg_offset;
guint8 seg_length;
}bits_spec_t;
e.g. the spec for the BSN field in an EGPRS DL type 3 header (3GPP
44.060 Figure 10.3a.3.3.1) is:
static const bits_spec_t bits_spec_dl_type3_bsn1[] = {
{15, 1},
{8, 8},
{0, 2},
{0, 0}
};
I have got a working version (for values of up to 32 bits spanning up to
32 bits of tvb), I am now doing some more generalisation to give it the
capability of extracting up to 64 bits,
spanning a larger portion of a tvb. This will require a little bit of
work to some existing helper functions.
I will submit a patch in due course.
regards,
Mike
