ronnie sahlberg wrote:
Interesting.
Four comments:
1, One would have to be a bit careful with where one uses
proto_tree_is_null() since there are instances where one protocol
dissector calls functions/subdissectors from other protocols. For
example NLM will call the dissector for NFS filehandles from the NFS
dissector. This would need to be documented. Can you write a README
for discussion on this?
I'll look at it.
2, proto_tree_is_null() would need a better more appropriate name.
:)
3, comment in your patch: i really think you need to do refcounting
for these items. I am not sure, but one might do refcounting just to
be on the safe side.
The refcounting might be as easy to implement as just
incrementing/decrementing hfinfo->infilter and letting ==0 mean not
used at all in any filter. Would need to check that that would work.
infilter is a tristate -1 is used for filtering on protocol type. Could
use some ugly bitfield though
4, proto_tree_is_null() is a too blunt instrument, it either enables
all fields or none fields. In addition to that one might also consider
putting
if(!( ->visible or ->infilter ))return
at the beginning of the proto_tree_add_ functions as well to further
reduce the number of calls made.
This might reduce the filter overhead even further.
Yes but:
1) it will affect all dissectors, no idea if it's good or bad
2) not sure about tap stuff.
3) you can't do it at the beginning because then you return a NULL
pointer and it breaks stuff like
ti = proto_tree_add_item(tree, hf_val1 ...
/* ti is NULL */
s_tree = proto_item_add_subtree(ti, ett);
/* s_tree is NULL */
proto_tree_add_uint_hidden(s_tree, hf_val2...
/* oops if you need hf_val2*/
and from a vgprof output most of the time (ok it's the instruction count
but ethereal is so hard on the cache that it's not a too bad proxy) is
in building and destroying tree nodes.
Didier
