Re: Enabling/disabling ANY heuristic dissector

[Hadriel Kaplan <hadrielk () yahoo com>]

> On Jul 5, 2015, at 7:02 PM, mmann78 () netscape net 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 <wireshark-dev () wireshark org>
Archives:    https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request () wireshark org?subject=unsubscribe