Wireshark-bugs: [Wireshark-bugs] [Bug 7881] Plugin KNXnet/IP and cemisubdissect

Date: Sat, 20 Oct 2012 17:16:52 -0700 (PDT)
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=7881

Michael Mann <mmann78@xxxxxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mmann78@xxxxxxxxxxxx

--- Comment #1 from Michael Mann <mmann78@xxxxxxxxxxxx> 2012-10-20 17:16:52 PDT ---
Dissectors should be submitted as a patch.  It's also prefered to submit
dissectors as built-ins, rather than plugins. 

Please see 
http://www.wireshark.org/docs/wsdg_html_chunked/ChSrcContribute.html

for general submitting guidelines.

General code comments:

1. Please use only C style comments
2. proto_register_cemi should be at the end of the file.  You can also
construct the function order so you don't need function declarations at the top
of the file.
3. Are all of the #defines really necessary if they are just for populating
value_strings?  You can just have the numeric value in the value_string itself.
4. A consistent whitespacing format needs to be applied.
5. Needs to have Wireshark "license" template at top of dissector files

packet-cemi.c
1. cemi_ll_length_table doesn't appear to be used.
2. tvb_memcpy() shouldn't be used for byte values.  Use tvb_get_guint8()

packet-knxnetip.c
1. Clean up commented out code in proto_reg_handoff_knxnet_ip()
2. tvb_memcpy() shouldn't be used for integer values.  There are tvb_get_*
functions that handle endianness for you.

-- 
Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.