In working on bug 3534, one of the review comments was that a "tree != NULL check" placed near the start of the dissector should be removed as it prevented some of the COL_INFO/expert info data from being populated if tree was NULL. While I agree the "tree != NULL" check should probably be moved to just after setting the COL_PROTOCOL column (and clearing the COL_INFO column), I didn't think it should be removed entirely. This prompted the following response from Jeff Morris:
"Yes, the tree==NULL check is for faster processing. Some people care very much
about speed [probably because they often work with larger files]. Wireshark
with no coloring rules (how I normally work) or tshark (without "-V") both
stand to benefit from if(tree) checks.
But it's all a question of effort vs. reward. If you've got a function with
100 proto_tree_add_item()s that don't affect the columns or expert info then
you could easily trade one if(tree) for 100 function calls which then end up
taking the shortcut out when tree==NULL. OTOH if you've got column updates and
expert infos hiding in there (or if you're tracking the offset into the tvb)
then you're probably better off without the if(tree) check."
I guess I've always used the rule that simple [1] dissectors (no matter how large) should all have the tree != NULL check before any dissection really takes place. Most of the "expert info" I've seen is attached to "tree items" along the lines of "field validation" (command/value not supported/recognized, length incorrect, etc). Without the tree, they don't seem very useful. I've also seen dissectors that appear to be more geared towards tshark (lots of data in COL_INFO) than Wireshark, as I assume that's the developer's preferred "output". I guess I'm looking for more hard and fast rules to apply that will equally benefit Wireshark and tshark users (minimizing effort, maximizing reward). Is the "tree != NULL" check something I should probably just avoid altogether? For "not simple" dissectors, I definitely agree having lots of "tree != NULL" checks isn't worth the effort.
[1] I consider a dissector without things like conversation data, fragmentation, "internal" states, or the need for pinfo->fd.visited, etc. to be "simple".
