Wireshark-bugs: [Wireshark-bugs] [Bug 2688] Mojito Protocol Dissestor Plugin

Date: Mon, 28 Jul 2008 17:03:58 -0700 (PDT)
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=2688





--- Comment #15 from Jeff Morriss <jeff.morriss.ws@xxxxxxxxx>  2008-07-28 17:03:56 PDT ---
(In reply to comment #14)
> It was not actually embedded in the fucntion, just an indenting issue my editor
> likes to do at times. Its fixed now so you  can better see the fact that it is
> in fact two separate functions (not one embedded in the other.)

Oh, er, yeah, sorry 'bout that.  (Just proves I was looking at the code in
Firefox instead of my editor.)

A few other comments:

- port 4001 is not IANA assigned so this dissector really should be a "new
style" one: that is, it should do some heuristics and if it finds the packet
doesn't belong to it, it should return 0 (it also would need to register with
new_create_dissector_handle()--see README.developer)

- More (sub-)functions, please!

- I noticed a couple cases where the values "IPV4" or "IPV6" were used instead
of the #defines

- A few switch statements have no default case--is that really the behavior you
want if the packet is wrong/munged?  (The answer may be yes but I just wanted
to make sure you'd thought of it; this may also relate to the
it-can't-be-Mojito comment)

- by the time you get to this code:

        if (!initialized)               {
                mojito_handle = create_dissector_handle(dissect_mojito,
proto_mojito);
                initialized = TRUE;
        }               else                    {
                dissector_delete("udp.port", global_mojito_port,
mojito_handle);
        }

        dissector_add("udp.port", global_mojito_port, mojito_handle);

'global_mojito_port' will already have the new value--so the delete won't work.
 That's why you need a 2nd variable (check some of the other dissectors for
examples.


(That's all I had time for now; one other thing I didn't have time to finish
looking at: it looks like there is only 1 possible subtree (ett_mojito) which
makes for an awfully flat tree.  But maybe that's the way the protocol is?)


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