Wireshark · Wireshark-bugs: [Wireshark-bugs] [Bug 2794] Questionable display filter fields

[bugzilla-daemon@xxxxxxxxxxxxx]

https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=2794

--- Comment #10 from Michael Mann <mmann78@xxxxxxxxxxxx> 2011-04-13 09:58:08 PDT ---
(In reply to comment #6)
> (In reply to comment #3)
> > I wrote a perl script 
> Automatic checking is definitely better than manually checking.  Thanks for
> writing it.  Hopefully it helps.
> > Current stats:
> > 16212 "questionable" display filters from 314 files
> There are an awful lot of them, but 16212?  My original spreadsheet, while
> compiled manually, "only" listed about 6315 of them.  Was I really that far
> off?  Or have there been 10K additions since 1.0.1 was released?  Can you
> confirm that number?

Yes I can, it was part of validating my script to begin with.  You have things
like packet-ncp2222.c that seems to be part of the "ncp" protocol
(packet-ncp.c) that has 2268 display filters for "ncp" not "ncp2222".   Numbers
also really add up for "category 3" when each dissector has 40-70 display
fields.  In terms of "display filters that really need to be fixed" your 6300
is probably more accurate.

Also, the perl script isn't that thorough with "Fields with apparent
redundancies", so that may bump the number up (as I briefly compared your
spreadsheet to mine)


> > Popular categories for the "questionable" display filters include:
> > 2. swapping '-' for '_' between PROTOABBREV and the display filter name
> Personally, I think a '.' should be between PROTOABBREV and the display filter
> name, not a '-' or '_'.

Example: packet-aim-messaging.c (PROTOABBREV = aim-messaging) having all
display filters of "aim_messaging."   Personally I think the fix should be
renaming the file to packet-aim_messaging.c.  If all the display filters in a
dissector have this formatting, this is how you get to 16000 "questionable"
display filters.

> > I'd like opinions on what to do with the list and how to proceed.  #1 is self
> > explanatory, but #2 and #3 require file name changes, which really don't work
> > well as a contributed patch (but I can compile a list of just the ones that
> > fall in those categories).
> Maybe we should just start with the obvious typos.  That alone should clear up
> a lot of them.  I don't think we're going to rename any files just for this. 
> PROTOABBREV != filename, necessarily.

If a patch is created for just the typos (I really don't think it would be that
big), should a high priority be placed on the patch, so the SVN doesn't get to
far without incorporating it?  The bug itself still seems accurate with a
"normal" priority.

> > I was also unclear of the offical rules for when to use periods in a filter. 
> > Once the PROTOABBREV is met and a period followed, there was a lot of
> > inconsistency as to when they were applied.  If this bug will drastically
> > change the display filters, might as well go all the way.
> Well, there are no "official" rules per se ... at least not yet.  My own
> personal naming convention is to use a period for every new subtree and that
> periods aren't used in display filter names for any other purpose.  But that's
> far from "official".

I agree with your interpretation of periods for new subtrees and/or
"subdissectors".  But at this point, I don't think the perl script needs to be
modified to enforce that or any other rules at this point (but it could be).  I
originally started with checkhf.pl, but since I'm new to perl reading regular
expressions took way too much time to figure out how to wedge my feature into
it, so I just copied it and replaced all of the "looking for hf" logic with my
own.

-- 
Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.