It is my understanding that for performance reasons, we surround calls to proto_tree_add_xxx with checks to see if the tree object is null prior to calling. I believe this is for performance reasons to prevent the overhead of the function call.
I'm starting to wonder though -- is this worth it? Just hear me out.
Functions like proto_tree_add_uint allow us to not only load the value into the tree, but to store the value into a variable as well. I think this is a very useful feature. However, because of us preventing the proto_tree_add_xxx calls from occuring when the tree is not being populated, this feature is pretty much useless. We would have to use tvb_get_ntohs (or some equivalent) in order to do further dissection anyway.
Further, if we want to store the value, AND display it, we are required to keep very careful track of the TVB offsets. We have to examine the code twice, once to store the variables, then we have to reset the offset and go over the code again to display the values in the tree.
I can understand the performance incentive in wanting to avoid the function call. But it seems that as a result we end up with alot of redundant code that could easily lead to inconsistent behavior.
If we are striving for correctness, it would seem that we should be working to keep the dissectors as simple as possible. This means avoiding doing things twice as much as possible.
It's not that I'm lazy, or that I'm concerned about code size. I just think that it is VERY easy to make mistakes when you have to write code to pass over the code once to dissect it and again to display it.
So here's the question: How much overhead do these calls produce? Would it be feasible to just store the value and return if tree is NULL?
If this has been beaten to death, then please note that I'm not trying to start a flamewar. I just think it's possible that in modern hardware this performance hack may not be worth the trouble, and we can dramatically reduce code size and complexity if we were willing to take the performance hit.
Thanks,
Devin Heitmueller
Senior Software Engineer
Netilla Networks Inc
