Re: [Wireshark-dev] [Wireshark-commits] rev 39143: /trunk/epan/dissectors/ /trunk/epan/dissectors/: packet-dvbci.c

["Maynard, Chris" <Christopher.Maynard () GTECH COM>]

> -----Original Message-----
> From: wireshark-dev-bounces () wireshark org [mailto:wireshark-dev-
> bounces () wireshark org] On Behalf Of Guy Harris
> Sent: Tuesday, September 27, 2011 3:37 PM
> To: Developer support list for Wireshark
> Subject: Re: [Wireshark-dev] [Wireshark-commits] rev 39143:
> /trunk/epan/dissectors/ /trunk/epan/dissectors/: packet-dvbci.c
>
>
> On Sep 27, 2011, at 12:11 PM, Maynard, Chris wrote:
>
> > I suppose, although I still question the real need for ENC_NA at all.
>
> At this point, I'm willing to see it go away, too.

Hmm, "willing to" != "prefer to".  If you of all people would prefer to keep it then I think we should definitely keep 
it.

> > Now that being said, changing the "const gboolean little_endian"
> argument to an "int endian" or better yet, "int encoding" argument
>
> $ egrep encoding epan/proto.h
>
>       ...
>
>     const gint start, gint length, const guint encoding);
[etc.]

Well what's the expression?  I guess this is me not seeing the forest through the trees here.  I was so focused on 
ENC_NA == ENC_BIG_ENDIAN, I didn't even notice the other ENC's, nor the guint encoding.  I also happened to read other 
things like, "In the future" from README.developer, so I didn't even think to look.  I do apologize for this.  But 
looking at the code and relative lack of use of these other ENC's, and noticing what's been changing (mostly TRUE -> 
ENC_LITTLE_ENDIAN, FALSE -> ENC_BIG_ENDIAN, and some ENC_NA's), it does seem that others might have been unaware of the 
other encodings as well?  I don't know, maybe there's been some confusion in thinking that the last argument to 
proto_tree_add_item() still meant only the endian used and so ENC_NA only meant that the endian-ness was not applicable?

OK, well that confusion aside, and assuming we want to do *something* to try to avoid situations where ENC_NA is 
incorrectly used where ENC_BIG_ENDIAN or [especially] ENC_LITTLE_ENDIAN should be, how does something like the attached 
patch grab you?  Keep in mind that this is a VERY INCOMPLETE patch (especially proto.c) and obviously needs more work; 
it's just to get the basic idea across.

So for example, in theory if changes such as this were to be made, then calls to proto_tree_add_item() that today don't 
warn us of any problems might be able to in the future.  Such as:

    Warning: TRUE/FALSE deprecated.
    proto_tree_add_item(tree, hf_ft_le_uint32, tvb, offset, len, TRUE);
    proto_tree_add_item(tree, hf_ft_be_uint32, tvb, offset, len, FALSE);

    Warning: Endian not applicable for this type.
    proto_tree_add_item(tree, hf_ft_bytes, tvb, offset, len, TRUE);
    proto_tree_add_item(tree, hf_ft_bytes, tvb, offset, len, FALSE);
    proto_tree_add_item(tree, hf_ft_bytes, tvb, offset, len, ENC_LITTLE_ENDIAN);
    proto_tree_add_item(tree, hf_ft_bytes, tvb, offset, len, ENC_BIG_ENDIAN);

    Warning: Invalid endian for this type.
    proto_tree_add_item(tree, hf_ft_le_uint32, tvb, offset, len, ENC_NA);
    proto_tree_add_item(tree, hf_ft_be_uint32, tvb, offset, len, ENC_NA);

- Chris

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.

Attachment: proto_diffs.txt
Description: proto_diffs.txt

___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
Archives:    http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request () wireshark org?subject=unsubscribe