Re: heur_dissector_add()

[Peter Wu <peter () lekensteyn nl>]

Hey David,

On Thu, Mar 22, 2018 at 06:32:08PM +0100, David Aggeler wrote:
> Hi Peter,
>
> Thanks for the hint.  Ok. I just debugged and apparently the DICOM one as many others is DISSECTOR_TYPE_SIMPLE. Not 
> sure how to change, but also not sure whether it is that relevant. However, I return 0 when it does not match (more 
> like new style)
>
> If the dissectors are combined like this, basically old and new need to follow the same interface, otherwise every 
> parent of call_dissector_through_handle() would need a type selector, and  I've not seen that anywhere else. 
>
> Also from this comment & code, I'd conclude that the retuned number is the number of processed bytes.
> In that sense, for me, 0 maps to FALSE in the heuristic and >0 maps to TRUE. 
>
> I've now coded according to the README.heuristic. To me that reflects 0/FALSE, >0 TRUE.
>
> > > you can enable or disable heuristics dissectors it in the Enabled Protocols menu.
>
> I'm blind. All I can see there is Ethernet PW (CW heuristic). That one?

The one of DICOM appears to be named "dicom_tcp" ("DICOM over TCP").

Kind regards,
Peter

> Regards
> David
>
> -----Original Message-----
> From: Wireshark-dev <wireshark-dev-bounces () wireshark org> On Behalf Of Peter Wu
> Sent: Thursday, March 22, 2018 17:29
> To: Developer support list for Wireshark <wireshark-dev () wireshark org>
> Subject: Re: [Wireshark-dev] heur_dissector_add()
>
> Hi David,
>
> On Thu, Mar 22, 2018 at 11:50:07AM +0100, David Aggeler wrote:
> >  
> >
> > I'm intending to re-enable the heuristic part in the DICOM dissector. 
> > So I read though the updates readme and some other dissector, and to 
> > my surprise, the return value of the heuristic still is supposed to be 
> > boolean, where the static one returns int.
> >
> >  
> >
> > Implementation wise, by now I kind of only see 'return 
> > tvb_captured_length(tvb)'. Wasn't this consumed bytes or needed bytes 
> > at some point? I used to return the same int also in heuristic part 
> > and never had an issue, but it looks wrong.
>
> There are only two conventions for the integer return value, see the comment for call_dissector_through_handle in 
> epan/packet.c:
>
>     /* This function will return
>      * old style dissector :
>      *   length of the payload or 1 of the payload is empty
>      * new dissector :
>      *   >0  this protocol was successfully dissected and this was this protocol.
>      *   0   this packet did not match this protocol.
>      *
>      * The only time this function will return 0 is if it is a new style dissector
>      * and if the dissector rejected the packet.
>      */
>
> When I tried to change this such that the returned value is actually significant 
> (https://www.wireshark.org/lists/wireshark-dev/201406/msg00221.html), I found that it would be quite risky to use the 
> return value to signal reassembly.
>
> > I did not understand that 8 years back, and I still don't. Does it 
> > mean a heuristic can't re-assemble?
>
> Heuristics can request reassembly, but then they must not reject the data (return true). But only do this when you 
> are sure it is your protocol. To ensure that your normal (not heuristics) dissector is called in the future, you can 
> use conversation_set_dissector.
>
> > The other part that seems to have changed are the settings for this. 
> > Is it not desired anymore, that the use can select at dissector level, 
> > whether it shall do the heuristic math or not?
>
> Rather than having a preference at every dissector, it is now taken of by in the core (see function 
> "heur_dissector_add"). In the GUI, you can enable or disable heuristics dissectors it in the Enabled Protocols menu.
> --
> Kind regards,
> Peter Wu
> https://lekensteyn.nl
___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
Archives:    https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request () wireshark org?subject=unsubscribe