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

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

A slightly different (and similarly incomplete) patch that uses 1 less bit of the encoding for endian determination, 
with the trade-off of not being able to tell the difference between TRUE and ENC_LITTLE_ENDIAN or FALSE and 
ENC_BIG_ENDIAN, in case that doesn't really matter and we'd rather reserve the bit for another purpose.


> -----Original Message-----
> From: wireshark-dev-bounces () wireshark org [mailto:wireshark-dev-
> bounces () wireshark org] On Behalf Of Maynard, Chris
> Sent: Friday, September 30, 2011 3:44 PM
> To: 'Developer support list for Wireshark'
> Subject: Re: [Wireshark-dev] [Wireshark-commits] rev 39143:
> /trunk/epan/dissectors/ /trunk/epan/dissectors/: packet-dvbci.c
>
> > -----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_diffs2.txt
Description: proto_diffs2.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