Wireshark-bugs: [Wireshark-bugs] [Bug 5929] New protocol dissector for "CIP Motion"

Date: Fri, 24 Jun 2011 07:09:31 -0700 (PDT)

--- 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.