Wireshark-bugs: [Wireshark-bugs] [Bug 9217] STANAG 5066 DTS Layer dissector

Date: Thu, 03 Oct 2013 22:22:32 +0000

Comment # 7 on bug 9217 from
Thanks all for your invaluable comments;

@Alexis:
The address is IPv4 ? why don't use tvb_get_ipv4 ? (or better  tvb_ip_to_str)
Not IPv4, they are STANAG 5066 node addresses and size of the address is not
constant. Thus the mentioned routines couldn't be used unfortunately.

@Michael: 
Q1. Do all of the functions really need forward references?  Can't the code be
constructed so that function definitions are before their use?

In effect, they don't. I've cleared the unnecessary function references.

Q2. Looks like this dissector should be a "new" style dissector and return 0 if
the packet doesn't look like it's a STANAG 5066 one.

You're right, I've modified the dissector function calls, register_dissector
and create_dissector_handle, into the new ones and added the return expressions
as well.

3. Remove #include <stdio.h> since it's defined out anyway.
Ok :)

4. Some of the proto_tree_add_text calls can be converted to expert info API
(ex. in dissect_s5066dts_eow_hftrp())

Ok, error messages are converted to expert api calls.

5. I don't believe columns need to be explicitly cleared before they are set.

You're right again, I've removed all unnecessary col_clear function calls.

6. Please put crc routines into epan/crc*-tvb.h
I realized that CRC look-up tables were created in wsutil/crc*.h for certain
polynomials. Am I supposed to put the routines directly to the relevant cry*.h
file existing in the directory 'epan' or dump the CRC lookup table of the
specified poly in STANAG 5066 and put it into amidst other tables under a
related wsutil/crc* file with the checksum computation routine in there?

@Guy:
Get rid of the preference for "encapsulated in TCP" vs. "directly captured from
the line" and, instead, have separate dissectors for those cases...
You want me to have different dissectors registered separately and both of
which has its own preferences, header fields, handles etc. Am I right?

Thanks all.


You are receiving this mail because:
  • You are watching all bug changes.