Wireshark-bugs: [Wireshark-bugs] [Bug 1443] Enhancements to configuration of sub-dissectors usin
Date: Wed, 23 Jan 2008 17:00:11 +0000 (GMT)
http://bugs.wireshark.org/bugzilla/show_bug.cgi?id=1443





--- Comment #5 from Doug Pratley <douglas.pratley@xxxxxxxxxx>  2008-01-23 17:00:03 GMT ---
Hi Stig

Glad you found this patch interesting. I wrote it to meet some specific
requirements we had last year at work, but since then I have been moved on to
other things at work; I also got married, which reduced my spare time, although
I'm not complaining.

The version I produced "works" but is rough around the edges, as you noticed. I
hoped that if I released it I'd get some feedback rather than trying to perfect
it in isolation. Well, it's been nearly a year, but I got some :-)

If you have a new version that compiles, please feel free to upload it: I'll
then download it and take a look to remind myself what I did. I don't know how
much time I'll have to do any serious work, so I'd be delighted if you're able
to improve the code - I'll try to help as much as possible.

To answer your specfic points:

1) wcslist
I wanted a list that was GTK version independent (other list-using code I
looked at in Wireshark was full of #ifdefs). So I created an interface,
wclist.h, and wrapped the GTK1 clist. All the code elsewhere in the patch just
uses the interface. Two problems
(a) There is no overall comment in the header file explaining this
(b) I never got around to implementing a GTK2 version of the internals - I
decided to get something out first.
So yes, ideally there should be one big "ifdef" with a GTK1 and GTK2 version of
the code in wcslist.c

2) Unable to "edit" the filter in an existing decode
This is because the filter is the "key" defining the decode (and is used as
such in the tables that hold the decode). To change the filter is really to
remove the existing decode and add a new one. 
It should be possible to extend the dialog code to do this behind the scenes
rather than forcing the user to do it explicitly.

3) Limited set of parent protocols
To use this feature, a parent dissector has to declare a table for
filter-specified decodes (just like the ones that use "ports" to distinguish
sub-dissectors). The only protocols ones I initially added one to were the ones
listed. The new table could be added to other protocols that have
sub-dissectors, but only one at a time.
It might have been better to extend the existing dissector table structure
rather than adding a completely new structure; that way all parent dissectors
that already used a "port" based approach to sub-dissectors might be able to
get the functionality for free? I was reluctant to risk changing something so
fundamental though...

4) Confusing duplicate entries in "Decode As" list
Yes, this is confusing - it's why I display the "parent key" in the "Add"
dialog. Unfortunately, there doesn't seem to be a unique name for some
dissectors... ideas welcome!

5) Select newly added protocol in the "Decode As" dialog
Sounds sensible.

6) Resolve OIDs in the "Parent Key" list
Sounds good - I have to admit I don't really understand what's going on with
those dissectors.

7) Add help buttons
Yes.


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