Wireshark-bugs: [Wireshark-bugs] [Bug 3443] A new dissector for the DTN Bundle Protocol

Date: Tue, 19 May 2009 23:09:13 -0700 (PDT)
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=3443





--- Comment #9 from Jaap Keuter <jaap.keuter@xxxxxxxxx>  2009-05-19 23:09:06 PDT ---
More review.
- The helper functions must be made static to avoid namespace pollution.
- analyze_tcp_convergence_layer_stream() doesn't exist, then don't prototype.
- tcp convergence layer has no preferences, don't register a module.

/*
 * These need to persist over multiple calls to the dissector to allow for the
 * multi-segment case.
 */
That won't work if you go random access or multi threading or have mixed
streams in the capture. If you really need these (which I somewhat doubt) this
has to be implemented within the context of a conversation.

in dissect_tcp_bundle()
    while(tree && (frame_offset < buffer_size)) { 
No reassembly when there's no tree? Then tshark won't work, neither will the
column info if colouring is off.

in dissect_udp_bundle()
    if(tree) { 
No column info will work if this check is in. See above.

It is customary to have the hf and ett array inside proto_register_xxx() and
have both proto_register_xxx() and proto_handoff_xxx() and the end of the
sourcefile. See template in README.developer.

     if (proto_bundle == -1) {
No need to check for this, proto_register is only called once.

    /*
     * This is weird but evidently what you have to do to get port prefs to
work.
     */
This statement has to go, it's confusing. The used method *is* the way to
combine protocol handoff and preference change in one function.


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