Wireshark-dev: Re: [Wireshark-dev] [Wireshark-commits] rev 38106: /trunk/epan/dissectors/ /trun
From: "Maynard, Chris" <[email protected]>
Date: Fri, 22 Jul 2011 17:23:08 -0400

My only problem with ENC_NA is that it is defined exactly the same as ENC_BIG_ENDIAN.  If you use ENC_NA in little-endian protocols, it is inevitable that “copy-and-paste” will occur at some point and then these endian bugs will start popping up because an ENC_NA was used where an ENC_LITTLE_ENDIAN should have been.


If ENC_NA was defined differently from ENC_BIG_ENDIAN, and if checks were made to be sure only valid ENC_*’s were passed to the proto_tree_add_xyz() functions for the particular FT_* field type, then I think it would make more sense (to me at least).  Right now, I still prefer to use ENC_LITTLE_ENDIAN throughout a little-endian protocol and ENC_BIG_ENDIAN throughout a big-endian protocol because it does no harm to do it and helps avoid future “copy-and-paste” bugs.  I just don’t see any particular advantage with using ENC_NA right now.  (Note that I only say that’s my personal preference; there seem to be more folks preferring ENC_NA, so I will use it too, I guess.)



From: [email protected] [mailto:[email protected]] On Behalf Of Alexis La Goutte
Sent: Friday, July 22, 2011 5:03 PM
To: Developer support list for Wireshark
Subject: Re: [Wireshark-dev] [Wireshark-commits] rev 38106: /trunk/epan/dissectors/ /trunk/epan/dissectors/: packet-bacapp.c



On Thu, Jul 21, 2011 at 9:45 PM, Bill Meier <[email protected]> wrote:

On 7/21/2011 3:35 PM, Jeff Morriss wrote:

Guy Harris wrote:

On Jul 18, 2011, at 10:19 PM, Maynard, Chris wrote:

Assuming the reporter of bug 5769 is correct and the Info column
displays the values of the low and high limits correctly, then the
protocol is ENC_BIG_ENDIAN. All of the fields affected by r38106 are
either FT_UINT8's or FT_BOOLEAN's spanning 1 byte, so endian-ness
really doesn't matter, but if someone does the old "copy-and-paste"
thing later on, [s]he might incorrectly copy an ENC_LITTLE_ENDIAN
when it should be ENC_BIG_ENDIAN.

If they're all one-byte fields, they might as well be ENC_BIG_ENDIAN,

I had wondered about that when converting some dissectors to ENC_XXX. I
ended up using ENC_NA for one-byte fields since it really is "not
applicable," but I'm not sure that was the Right thing to do. If all the
other fields are ENC_BIG_ENDIAN that I guess it would make sense to just
keep using that, even for one-byte fields.


My somewhat mild preference is to use ENC_NA for one-byte fields....




Why not add a check in checkhf.pl? (if proto_tree_add_item end by ENC_NA /ENC_BIG_ENDIAN / ENC_LITTLE_ENDIAN)



CONFIDENTIALITY NOTICE: The contents of this email are confidential
and for the exclusive use of the intended recipient. If you receive this
email in error, please delete it from your system immediately and
notify us either by email, telephone or fax. You should not copy,
forward, or otherwise disclose the content of the email.