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 <[email protected]> On Behalf Of Peter
> Wu
> Sent: Thursday, March 22, 2018 17:29
> To: Developer support list for Wireshark <[email protected]>
> 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 <[email protected]>
Archives: https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
mailto:[email protected]?subject=unsubscribe
