Wireshark · Wireshark-dev: Re: [Wireshark-dev] [Wireshark-commits] rev 22586: /trunk/epan/dissectors/ /trunk/epan/dissectors/:

[Guy Harris <guy@xxxxxxxxxxxx>]

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.)