I've had a play with this function, I like it - it can improve and
simplify parts of packet-umts_fp.c which are bit-oriented, like E-DCH
data. Having the function spit out the return value is helpful here,
as this will avoid the dissector doing the equivalent shiting and
masking work for a second time.
Having said that, although it shows the correct bytes for me, it is
computing the wrong value at the moment. For example, I selected 6
bits, offset 4 bytes into the bytes 0x4917. It shows the correct
bits, i.e. .... 100101.. .... but computes the value to be 101,
i.e. the bits 1100101. So it looks like maybe the masking is out by 1
bit - I'll look at it tomorrow if no-one else does first.
Best regards,
Martin
On 4/24/07, Anders Broman <a.broman@xxxxxxxxx> wrote:
-----Ursprungligt meddelande-----
Från: wireshark-dev-bounces@xxxxxxxxxxxxx
[mailto:wireshark-dev-bounces@xxxxxxxxxxxxx] För Ulf Lamping
Skickat: den 24 april 2007 23:05
Till: Developer support list for Wireshark
Ämne: Re: [Wireshark-dev] [Wireshark-commits] rev 21556: /trunk/epan/
/trunk/epan/: proto.c proto.h - all buildbots red now :-(
Anders Broman wrote:
>> Hi,
>> I have no problem with backing out my changes but a
>> Proto_... function should not be local to iuup in my opinion.
>> Perhaps it should be renamed iuup_proto_.. instead until
>> We have "the real thing" in proto.c
>>
>> What do others think?
>>
>Well, first of all, we shouldn't have a buildbot going into deep red for
>a long time, so there's something to be done.
>I'll fully agree that a function starting with proto_ shouldn't be in
>any dissector code - a name clash will be the result sooner or later -
>and today is sooner ;-)
>Could you please:
>1.) fix the issue in iuup by prepending iuup_ so the buildbot get's
>green again - I'm currently waiting for it :-(
Joerg beat me to it.
>2.) if your new function doesn't follow the common function pattern, fix
>this issue in your implementation (I didn't had a look at the code in
>proto myself)
Well what pattern it should have is being discussed.
As I see it:
Alt
1) Leave it as it is and change other proto_add.. functions to behave
similarly.
1b) Leave the signature but rename it to something like
proto_tree_add_bits_ret_val() and add similar functions for other stuff.
2) use(proto_tree* tree, int hf, tvbuff_t* tvb, int offset, int bit_offset,
guint bits) as in iuup
3) use (proto_tree *tree, int hf_index, tvbuff_t *tvb, gint bit_offset, gint
no_of_bits, gboolean little_endian)
Other?
Regards
Anders
_______________________________________________
Wireshark-dev mailing list
Wireshark-dev@xxxxxxxxxxxxx
http://www.wireshark.org/mailman/listinfo/wireshark-dev
_______________________________________________
Wireshark-dev mailing list
Wireshark-dev@xxxxxxxxxxxxx
http://www.wireshark.org/mailman/listinfo/wireshark-dev
