[RFC] bug 6797 : proto: Add proto_tree_add_split_{uint, int} helpers for non contiguous int fields

[Sylvain Munaut <246tnt () gmail com>]

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.


> Would it be better to go for 64bits?

I used 32 bits because all the display "magic" is done by a
pre-existing helper decode_bitfield_value and it only handles 32 bits.


> Come to think of it would proto_tree_add_split_bits_item(tvb,tree, 
> start_bit_offset,num_of_bits,num_skip_bits,num_of_remaining_bits,encoding) make more sense?

See above: Sometimes the bits don't have to be taken "in order" ...


Cheers,

    Sylvain

Attachment: add_proto_tree_add_split_uint_int_helpers.diff
Description:

___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
Archives:    http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request () wireshark org?subject=unsubscribe