Anders Broman wrote:
Jeff Morriss skrev 2011-09-27 21:56:
Anders Broman wrote:
Anders Broman skrev 2011-09-26 18:25:
Stig Bj�rlykke skrev 2011-09-26 18:14:
On Mon, Sep 26, 2011 at 4:51 PM,<etxrab@xxxxxxxxxxxxx> wrote:
Log:
Get rid of check_col, while at it set ENC.
Directory: /trunk/epan/dissectors/
Changes Path Action
+35 -37 packet-dmp.c Modified
The remaining check_col in packet-dmp.c was left on purpose, because
we do not need this calculations if not having COL_INFO.
Should we really remove all occurrences of check_col?
Perhaps not, but how often isn't the info column present?
Thinking about it this raises the question
- is check_col deprecated or not?
Should lengthy code to compute column info be protected in some other
manner?
fd->visited? (!tree)? We only fill in the column info on the first
pass before the tree is built, right?
Actually a quick test indicates that we only build the column info on
the 2nd pass (for those frames which are currently displayed).
In other words, if you've got 10,000 frames of which 25 are displayed
in your Wireshark window, your dissector is called 10,000 times with
check_col(COL_INFO)==0 and 25 times with check_col(COL_INFO)==1. As
you scroll down the dissector is called again for the newly-visible
frames with check_col(COL_INFO)==1. (It does appear that Wireshark
does cache the column information; scrolling back or clicking on
earlier packets does not result in the column info being rebuilt, even
if the dissector is called again.)
Actually another case when column info won't be present is tshark when
doing statistics, etc., but not printing the packet details.
This suggests that check_col()'s should be left in--at least if they
are protecting us from doing some work, including val_to_str()'s.
Even though the check_col is built into the function calls them self s?
My point is that if you've got a bunch of code which runs BEFORE getting
into the col_*() routines (and this would include simple--but
potentially expensive if the value_string list is big--things like
val_to_str()) then it could be saved by leaving the check_col() in the
calling routine (i.e., the dissector).
IOW, in this code sequence:
col_add_fstr(pinfo->cinfo, COL_INFO, "%s ",
val_to_str(h1, chm_h1_message_type_acro_values, "Unknown"));
val_to_str() always does work, even if col_add_fstr() immediately bails
out because the column isn't being built.
I don't know how many value_strings are out there which are big enough
for this to matter. But I suspect--and sounds like Stig does too--that
there are times where the amount of work wasted might matter. This just
implies that check_col()'s shouldn't be completely deprecated but rather
made optional. Certainly there is no point in guarding this code with a
check_col() ;-) :
col_set_str(pinfo->cinfo, COL_INFO, "Unknown ");