Comment # 4
on bug 8313
from Evan Huus
A few more minor questions and issues:
- Why did you write add_length_data()? Instead of "add_length_data(length,
data)" can't you just do "*length + *data"?
- The last argument to any call to proto_tree_add_item should be one of the
ENC_ values (ENC_NA, ENC_BIG_ENDIAN, ENC_LITTLE_ENDIAN). I'm not sure which
version of Wireshark you've been developing against, but we haven't used
TRUE/FALSE for those fields in ages. (Similar point applies to
proto_tree_add_*_format calls).
- Modelines are great, but you don't need them at the top *and* the bottom of
your file. Just the bottom ones are fine. The link I provided is unclear on
that point, I admit; sorry about that.
- You mention in one of your comments that it's "like BER". I know Wireshark
has some BER support functions already (though I myself am not at all familiar
with them). Would using them help your dissector? See
epan/dissectors/packet-ber.h
- You seem to be using integers for booleans in a few places (eg 'close1' and
'close2', line 2361). Wireshark uses glib, which provides a 'gboolean' type
that can be assigned TRUE or FALSE. This may simplify some parts of the code.
- You have some magic numbers in switch statements (lines 2492, 2497, etc). It
would be better to #define them as usefully named constants, then use those
defines in the switch statement and in the corresponding value-string (line
153, etc.)
You are receiving this mail because:
- You are watching all bug changes.
