> On Jul 5, 2015, at 7:02 PM, [email protected] wrote: > > I uploaded a patch to Gerrit that allows enabling/disabling of any heuristic > dissector (https://code.wireshark.org/review/9508/). > > Some comments about the patch (others are welcome to add more): > 1. Not sure how to best express the relationship between the "name" of the > heuristic dissector and its "parent"/table name. For example, the AD-win > Config protocol has a heuristic dissector that goes on top of TCP and UDP. > Each instance (TCP or UDP) can be enabled/disabled separately. Requiring an > individual name for each heuristic dissector sounds like a bit too much to > ask. Right now they are slash (/) delimited in the GUI and comma delimited > in the "disabled_heuristics" file.
I think the slash delimiter in the GUI is ok - not great, but I can’t think of anything more clear. Perhaps have the Column title also be slash delimited, like “Heuristic Protocol/Underlying Protocol”? (or some better word for “Underlying”) > 2. These "preferences" are read right after the enable/disable protocols are > read/applied. This seemed like a logical place for it (since I added support > for enable/disable heuristics in disabled_protos.c), but I'm not sure how > that effects the "general" protocol preferences. Both manipulate the same > heuristic dissector list, so "last one would win”. I think all the individual preferences to enable/disable a heuristic should be removed. Are you talking about some other “general" protocol preference? One note: this would mean we can’t ship the Qt-based GUI until the Enabled Protocols dialog box is available in it. > 3. I'd like to remove any individual dissector preferences that > enable/disable a heuristic dissector (future patch) using the same > logic/justification as Decode As. Not sure how to handle heuristic dissectors > that should be off by default. heur_dissector_set_enabled(..., FALSE) can be > used, but that won't be "overridden" (enabled) by the fact that the heuristic > dissector ISN'T in the disabled heuristics file. I think we should change all the heur_dissector_add() function calls to take a boolean for whether it is enable by default or not; that way developers of future heuristic functions have to think about it and can’t forget. The heur_dissector_set_enabled() should be removed. > 4. I understand the "feature" of enabling/disabling a (heuristic dissector) > preference from the context menu, and that could be justification/argument > for keeping it (the preference). Maybe just "appending" the context menu > preferences for any protocol that has a heuristic dissector would be a good > compromise? You can currently disable a protocol itself with the context menu, without them having a preference for doing so, right? So I suggest we just follow that same behavior for heuristics, whatever that is. :) -hadriel ___________________________________________________________________________ Sent via: Wireshark-dev mailing list <[email protected]> Archives: https://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev mailto:[email protected]?subject=unsubscribe
