Wireshark-bugs: [Wireshark-bugs] [Bug 3238] patch to support ZIOP and MIOP (specialized CORBA pr
Date: Fri, 6 Feb 2009 13:24:59 -0800 (PST)
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=3238


Bill Meier <wmeier@xxxxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |wmeier@xxxxxxxxxxx




--- Comment #2 from Bill Meier <wmeier@xxxxxxxxxxx>  2009-02-06 13:24:54 PDT ---
Thanks for the submission ! 

A few small comments (others may have addt'l comments):

1. Please convert the C++ style comments to C-style. (Not all C compilers which
might be used to build Wireshark will accept C++ style comments).

2. Please put a $Id$ line as follows in each of the source file.
   (I'm not sure, but I think a line with just $Id$ should replace the 
   "$Id ...."  line which exists in some of the files.

 * $Id$
 *
 * Wireshark - Network traffic analyzer
 * By Gerald Combs <gerald@xxxxxxxxxxxxx>

3. The "standard dissector template" we've been using
   puts proto_register... and proto_reg_handoff... (in that order) at the 
   end of the source file. Although not necessary, it would be appreciated
   if you could make this change.

4. packet-miop.c
   - include <epan/prefs.h> is not required since there are no preferences.
   - For reg_handoff ...
      + The "if (!initialized) ... " is not req'd since reg_handoff will
        only be called once at Wireshark startup. (The use of
        'if (!initialized)...' is needed when the reg_handoff fcn is also
        used as a prefs callback).
      + The following is not req'd for handling conversations.
          /* Port will be set by conversation */
          dissector_add("udp.port", 0, miop_handle); 
        If the dissector is to be registered so that it is is available on the
        Decode_As menu, then it should be registered using
        dissector_add("tcp.port,....) rather than registering it on udp port 0.
   - The memory g_malloc'd for unique_id.id never seems to be freed;
     ep_alloc(...) can be used instead of g_malloc(...) so the
     memory will be freed automatically once the dissection of 
     the current frame is complete.

5. packet-ziop.c
   - The prefs and emem includes are not req'd;
   - The ziop_compressor_ids value string array should be null-terminated
     like the other value_string array.
   - For reg_handoff...
      + The "if (!initialized) ... " is not req'd since reg_handoff will
        only be called once at Wireshark startup. (The use of
        'if (!initialized)...' is needed when the reg-handoff fcn is also
        used as a prefs callback).
      + The following is not req'd for handling conversations.
          /* Port will be set by conversation */
          dissector_add("tcp.port", 0, ziop_handle); 
        If the dissector is to be registered so that it is is available on the
        Decode_As menu, then it should be registered using
        dissector_add("tcp.port,....) rather than registering it on tcp port 0.


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