Wireshark-bugs: [Wireshark-bugs] [Bug 5929] New protocol dissector for "CIP Motion"
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=5929
--- Comment #2 from Jaap Keuter <jaap.keuter@xxxxxxxxx> 2011-06-24 16:09:30 CEST ---
Quick review:
There's a lot of 'var = tvb_get_..(); proto_tree_add_text(..var);' going on.
This should not be required. Properly setup header fields allow for
proto_tree_add_item() to be used, with all its goodies.
'#define _CRT_NON_CONFORMING_SWPRINTFS' has to go.
C++ style comments have to go.
Mixed tab/multi space indentation has to go.
Commented out code has to be finalized, or marked, or removed.
add_byte_array_text_to_proto_tree() has to be looked at
Use of check_col() is deprecated.
NEVER EVER manipulate columns inside 'if (tree)' condition.
Preferred order at EOF is proto_reg_xxx(), proto_handoff(), then EOF
Was this source file header field checked?
Was this dissector fuzz-tested?
Can we have a sample capture file for testing?
--
Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
You are watching all bug changes.
