On Aug 22, 2007, at 3:19 PM, Stig Bjørlykke wrote: > My personal opinion is that this bitfields should be hidden in a > subtree, like this patch: > > - proto_tree_add_uint(ip_tree, hf_ip_frag_offset, tvb, offset + > 6, 2, > - (iph->ip_off & IP_OFFSET)*8); > + tf = proto_tree_add_uint_format(ip_tree, hf_ip_frag_offset, tvb, > offset + 6, 2, > + iph->ip_off, "Fragment offset: %d", (iph->ip_off & > IP_OFFSET)*8); > + field_tree = proto_item_add_subtree(tf, ett_ip_frag_offset); > + proto_tree_add_item(field_tree, hf_ip_frag_offset, tvb, offset + > 6, 2, FALSE);
That's adding one more layer, with what amounts to a copy of the value underneath it. Other than providing the raw offset, what advantages does it offer? (There, I think, are other dissectors that have bitfields that aren't in a subtree; if the word containing all the bitfields isn't specified as an item of its own in the protocol, I'm not sure it needs to be put into the protocol tree.) _______________________________________________ Wireshark-dev mailing list [email protected] http://www.wireshark.org/mailman/listinfo/wireshark-dev
