https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=5794
--- Comment #3 from Bill Meier <wmeier@xxxxxxxxxxx> 2011-04-28 19:36:31 EDT ---
Some comments ...
1. I'm pretty sure that ep_alloc instead of se_alloc should be used in the
following since the reversing buffer will be used only during the dissection of
the current frame:
...
len = tvb_length(tvb);
reversing_buffer = se_alloc(len);
memcpy(reversing_buffer, tvb_get_ptr(tvb, offset, -1), len);
...
Using se_alloc means that a new buffer will be allocated each time this
dissector is called and that the buffers will not be freed until
a complete re-dissection is done (filter changed, file re-opened,
new file opened, etc).
2. A thought:
Looking at the code quickly it appears to me that almost all of the code
can be under an 'if(tree)' so that, for instance, the presumably relatively
expensive creation of the reversing buffer is done only when really needed.
That is: only when a frame is actually displayed (or is being filtered,
etc).
One issue is that the dissector is a new-style dissector and thus
should return the number of bytes dissected which would presumably require
a complete dissection regardless of 'if(tree)'. Maybe it would be OK
for the dissector to be old-style ?
so: Would maybe something like the following work ?
void dissect_bmc(...) {
...
col_set_str(pinfo->cinfo, COL_PROTOCOL, "BMC");
col_clear(pinfo->cinfo, COL_INFO);
message_type = BIT_SWAP(tvb_get_guint8(tvb, offset));
col_add_fstr(pinfo->cinfo, ..., ..., val_to_str(message_type, ...);
if (!tree)
return;
...
return;
}
(or, maybe this approach is a non-starter given the comment about
"to do: add dissection of the CBS message payloads").
Comments ?
====
P.S. Can you provide a capture file for this protocol ?
--
Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
