Wireshark-dev: Re: [Wireshark-dev] Display Filter Fields typos, missing fields, etc.

From: Jaap Keuter <jaap.keuter@xxxxxxxxx>
Date: Thu, 23 Aug 2007 18:33:25 +0200
Hi,

As defined in README.developer:

        static hf_register_info hf[] = {
                { &hf_PROTOABBREV_FIELDABBREV,
                        { "FIELDNAME", "PROTOABBREV.FIELDABBREV",
                        FIELDTYPE, FIELDBASE, FIELDCONVERT, BITMASK,
                        "FIELDDESCR", HFILL }
                }
        };

So look up the PROTOABBREV and you'll be fine.

Thanx,
Jaap


Maynard, Chris wrote:
OK, I will start submitting bugs and patches for each one individually as I find time.

I'll use Guy's recommendation for naming fields, which is also my preference (even if I mistyped it earlier).  So, for a dissector like packet-aim-admin.c, all fields will be prefixed with "aim_admin.", and similarly for the rest.

- Chris

-----Original Message-----
From: wireshark-dev-bounces@xxxxxxxxxxxxx on behalf of Guy Harris
Sent: Wed 8/22/2007 6:00 PM
To: Developer support list for Wireshark
Subject: Re: [Wireshark-dev] Display Filter Fields typos, missing fields,etc.
On Aug 22, 2007, at 12:39 PM, Maynard, Chris wrote:

I was looking at the display filter fields recently and noticed that
there seem to be some display filter field naming mistakes/typos.
Basically, there are a lot of naming inconsistencies, and in at least
one case, no name at all. Here are a just a few of the dissectors that
seem affected:

packet-ieee80211.c:
1.	There are many fields (I count 51 of them) that seem to me that
they should be prefixed with "wlan.".  For example, fixed.chanwidth,
fixed.qosinfo.ap, fixed.qosinfo.ap.edcaupdate, ...

Yes, those are probably typoes.

2.	hf_tag_measure_rpi_histogram_report is missing a display filter
name, i.e.,
		{"Receive Power Indicator (RPI) Histogram Report", "",

That one probably is a typo/mindo, too.

packet-slowprotocols:
		There appear to be 4 different prefixes used: "slow.",
"lacp.", "marker.", and "oam.".  Should they all be prefixed with
"slow."?

Possibly. The Link Aggregation Control Protocol, the Link Aggregation Marker Protocol, and the Operation, Administration and Maintenance protocol are all arguably separate protocols; however, they don't have independent Ethertypes, they all use the "slow protocols" Ethertype, with the "slow protocols" header having a 1-byte subtype field specifying which particular slow protocol the packet contains. As such, perhaps the dissector should dissect only the subtype as the "slow" protocol, with separate dissectors for all the individual slow protocols.

packet-aim-admin.c:
		Two fields are prefixed with "aim." and one is prefixed
with "admin.".  Shouldn't all 3 be prefixed with, "aim.admin."?

Or, given that the protocol is "aim_admin", perhaps they should all be prefixed by "aim_admin.".

packet-gsm_a.c:
1.	"sm_a.rr.pow_cmd_pow" should probably be "gsm_a.rr.pow_cmd_pow".

Probably just a typo.

There are a lot more, but for brevity, I haven't listed them all here.
Is reporting this on the developer's list sufficient, or should I open a
bug for all/some of them?

I'd open bugs for each dissector (so we don't have to fix them all at once to close the bug).

Perhaps there should be a more strict naming convention?

Or a more clearly stated naming convention. The general convention is that a field name's first component should be the name of the protocol to which it belongs.

Is there a way to automate the names so typos, missing fields, etc. can be avoided?

With the existing scheme in which the field list is constructed by hand, about all you could do would be to use macros and compile-time concatenation, or do string processing at registration time; the latter might slow down registration a bit, and increase memory use.

Dissectors generated automatically could probably do better. There are a number of reasons why, in general, it's a Good Thing to generate dissectors from specification languages, and that's one of them.

Registration-time checking of the fields could also help - or, perhaps, instead of doing the checking all the time, just do it at build time, with a special option to tshark, or a special checker program (Did somebody already write something such as that? I vaguely remember something of that sort.).
_______________________________________________
Wireshark-dev mailing list
Wireshark-dev@xxxxxxxxxxxxx
http://www.wireshark.org/mailman/listinfo/wireshark-dev




-----------------------------------------
This email may contain confidential and privileged material for the
sole use of the intended recipient(s). Any review, use, retention,
distribution or disclosure by others is strictly prohibited. If you
are not the intended recipient (or authorized to receive for the
recipient), please contact the sender by reply email and delete all
copies of this message. Also, email is susceptible to data
corruption, interception, tampering, unauthorized amendment and
viruses. We only send and receive emails on the basis that we are
not liable for any such corruption, interception, tampering,
amendment or viruses or any consequence thereof.