Wireshark · Wireshark-dev: Re: [Wireshark-dev] [Wireshark-commits] rev 43176: /trunk/epan/dissectors/ /trunk/epan/dissectors/:

[Jeff Morriss <jeff.morriss.ws@xxxxxxxxx>]

Finally got back to this...

OK, so now I understand what was happening: the PDML generation code was 
trying to pull out the display value and, since it wasn't being found in 
the value_string (because it was an unexpected/fuzz'd value), it had to 
generate a numeric representation which it could not do because it had 
BASE_NONE.

So I added code to prevent registering such fields in r43412.

BUT, in writing this email, I'm now realizing this isn't quite right: 
construct_match_selected_string() has a special case where it will 
generate the string representation of a value rather than the value 
itself, but only when display is BASE_NONE.

In other words, prior to the recent changes if I used BASE_NONE for a 
field (say, sctp.chunk_type) then if I did "prepare as filter" for that 
field, I'd get, for example:

	sctp.chunk_type == "HEARTBEAT"

whereas using BASE_DEC would give me:

	sctp.chunk_type == 4

Now (in trunk and trunk-1.8) dissector writers no longer have that 
option.  I'm not convinced they should; maybe we should always do one or 
the other (which?).  Or we can give them the option and fix 
construct_match_selected_string() to handle the case when the strings 
function doesn't find a value (and thus drops through to the "get a 
numeric representation" code).  Needs more thought.  At the very least, 
the now-dead code at the beginning of construct_match_selected_string() 
should be cleaned up.

Maynard, Chris wrote:
> Also, the bug reporter indicated:
>
>         Unhandled exception ("proto.c:6698: failed assertion
>         "DISSECTOR_ASSERT_NOT_REACHED"", group=1, code=4)
>
> ... so while it was a guess, it was an educated guess, and it really seemed to me that this was the cause.
> - Chris
>
> ________________________________________
> From: wireshark-dev-bounces@xxxxxxxxxxxxx [wireshark-dev-bounces@xxxxxxxxxxxxx] On Behalf Of Maynard, Chris [Christopher.Maynard@xxxxxxxxx]
> Sent: Saturday, June 09, 2012 2:40 PM
> To: Developer support list for Wireshark
> Subject: Re: [Wireshark-dev] [Wireshark-commits] rev 43176: /trunk/epan/dissectors/ /trunk/epan/dissectors/: packet-ieee80211.c
>
> It was a guess.  The attachment in the bug report, namely "wint168.txt", only revealed the following::
>
>         This application has requested the Runtime to terminate it in an unusual way.
>         Please contact the application's support team for more information.
>
> I didn't see anything wrong with the "wlan_mgt.fixed.capabilities.dsss_ofdm" field, which was the one in which the above message appeared, so I went looking for nearby fields for potential problems, and that's when I noticed that hf_ieee80211_tag_supp_rates had "FT_UINT8, BASE_NONE", and the display filter for that field is "wlan_mgt.supported_rates", which is the last thing printed in the wint168.txt file, so I figured that was most likely the problem.
>
> - Chris
>
> ________________________________________
> From: wireshark-dev-bounces@xxxxxxxxxxxxx [wireshark-dev-bounces@xxxxxxxxxxxxx] On Behalf Of Jeff Morriss [jeff.morriss.ws@xxxxxxxxx]
> Sent: Saturday, June 09, 2012 2:26 PM
> To: wireshark-dev@xxxxxxxxxxxxx
> Subject: Re: [Wireshark-dev] [Wireshark-commits] rev 43176: /trunk/epan/dissectors/ /trunk/epan/dissectors/: packet-ieee80211.c
>
> On 06/09/2012 01:08 PM, cmaynard@xxxxxxxxxxxxx wrote:
> > http://anonsvn.wireshark.org/viewvc/viewvc.cgi?view=rev&revision=43176
> >
> > User: cmaynard
> > Date: 2012/06/09 10:08 AM
> >
> > Log:
> >   Do not use BASE_NONE for FT_UINT8 types.
> >   Fixes https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=7333 (I think).
>
> The code in epan/proto.c seems to indicate that using BASE_NONE with
> FT_*INT* types should be OK when there the strings converter is supplied:
>
> >                 case FT_UINT32:
> >                 case FT_UINT64:
> >                         if (hfinfo->strings == NULL) {
> >                                 /*  Require integral types (other than frame number,
> >                                  *  which is always displayed in decimal) to have a
> >                                  *  number base */
> >                                 if (hfinfo->display == BASE_NONE)
> >                                         g_error("Field '%s' (%s) is an integral value (%s)"
> >                                                 " without strings but is being displayed as BASE_NONE\n",
> >                                                 hfinfo->name, hfinfo->abbrev,
> >                                                 val_to_str(hfinfo->type, hf_types, "(Unknown: %d)"));
> >                         }
>
> Where was it crashing (er, excepting out)?  (There's no sample PCAP file
> in that bug.)