Wireshark-bugs: [Wireshark-bugs] [Bug 8505] openSAFETY: New reassemble functionality, as well as
Comment # 8
on bug 8505
from Evan Huus
A few questions and notes:
- read_lowhigh_order appears (at first glance) to simply be doing byte order
conversion from one endianness to another. We provide the functions
tvb_get_ntohl and tvb_get_letohl (depending on which endianness you need) for
precisely this purpose. The calculation of 'ssdoIndex' can similarly be
replaced with a call to tvb_get_ntohs or tvb_get_letohs.
- In general, duplicating the entire payload with ep_tvb_memdup is unnecessary
if all you're going to do is fetch certain values from it. There should be a
function to get any type of value you need directly from the tvb without any
manual bit-twiddling.
- tvb_reported_length is usually preferred over tvb_length in most cases, since
it permits proper dissection of truncated captures (the result of passing -s to
tcpdump, for example)
- You use mystery constants (such as 0x1018) in several places in the code.
Naming these with #defines is recommended (I think such names can be used in
your value strings as well?)
- The ./tools/checkhf.pl script reports one error:
Unused entry: epan/dissectors/packet-opensafety.c, hf_oss_ssdo_extpar_hdr
- There's one C++-style comment that slipped in accidentally (caught by
./tools/checkAPI.pl)
You are receiving this mail because:
- You are watching all bug changes.
