https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=2688
--- Comment #19 from Travis Dawson <travis.dawson@xxxxxxxxxx> 2008-08-28 20:02:47 PDT ---
(In reply to comment #18)
> (In reply to comment #16)
> > > - More (sub-)functions, please!
> > Not really a need to do this much, but I will put some of the contact tree
> > stuff in subfunctions.
I am working on adding some subfunctions to take care of repeated stuff.
> I'm not sure I get your comment. My point was that you've got some mighty big
> functions in there. Maybe it looks worse than it is due to the indenting
> problem you had before?
>
> > > - I noticed a couple cases where the values "IPV4" or "IPV6" were used instead
> > > of the #defines
> > IPV4 and IPv6 are defines.
Fixed
> > > - 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.
> >
> > I think I get it, can you point out a quick example so I know what it is
> > exactly.
>
> One example is packet-beep.c
Fixed this and added Hueristics for good measure :)
>
> > > (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?)
> > I have a bunch of subtree's under the main tree.
> > I think that's what your asking.
>
> OK, I see that now. But each branch in the tree "should" have its own 'ett'
> variable. For example here:
>
> /* Setup the Header Tree */
> mojito_headertree = proto_item_add_subtree(mojito_tree_temp,
> ett_mojito);
>
> you're reusing ett_mojito for the Header tree. That's not a big deal but it
> means that if you open the Mojito tree (but not the Header tree) and then move
> to another packet all the subtrees you have created will be opened. If you had
> multiple ett_ variables the user would get the same view (same trees
> expanded/opened) when going from packet to packet.
I like that behavior. If someone complains (its not policy or something) I will
change it, but I prefer that behavior actually.
--
Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
