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.
