Wireshark-dev: Re: [Wireshark-dev] New proto_add_bits function (Was: rev 21556:/trunk/epan//tr.

From: Richard van der Hoff <richardv@xxxxxxxxxxxxx>
Date: Tue, 15 Apr 2008 18:42:58 +0100
Hi all, and Anders in particular:

Anders Broman (AL/EAB) wrote:
Hi,
I have used it in the h263 dissector with a h263 prefix unfortunatly there is only short sequences
Beeing dissected most being the same in all h263 frames :(
Perhaps it can be used in the PER dissector but I didn't want to use it before we had agreed on a format to
avoid to many changes.
Regrads
Anders

I've just been investigating why packet colouring and display-filters weren't working on h263 picture headers. It turns out that the problem was that proto_tree_add_bits_ret_val extracts the value, returning 0 or 1 (for a 1-bit field); proto_tree_set_uint then applies the bitmask specified in the hf_field definition - giving 0 for most fields.

It seems to me that the bitmask should be 0 for all fields intended for use with proto_tree_add_bits_*.

I've fixed this for h263 in r25050, but I'm concerned that there will other cases, both now (h264, maybe?) and in the future - and that such bugs will once again lie dormant until somebody tries to filter on the offending fields.

I'd therefore like to propose adding this to proto_tree_add_bits_ret_val:

+       if(hf_field -> bitmask != 0) {
+ REPORT_DISSECTOR_BUG(ep_strdup_printf("Incompatible use of proto_tree_add_bits_ret_val with field '%s' (%s) with bitmask != 0", + hf_field->abbrev, hf_field->name));
+       }
+

Comments?

Cheers

Richard