Wireshark-dev: Re: [Wireshark-dev] [Wireshark-commits] rev 40877: /trunk/epan/dissectors/ /trun
From: Alexis La Goutte <[email protected]>
Date: Tue, 7 Feb 2012 08:31:56 +0100
Hi,

On Tue, Feb 7, 2012 at 8:20 AM, Joerg Mayer <[email protected]> wrote:
On Mon, Feb 06, 2012 at 04:36:23PM +0000, [email protected] wrote:
> http://anonsvn.wireshark.org/viewvc/viewvc.cgi?view=rev&revision=40877
>
>  From https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=6704
>  DNS KEY RDATA contains an extra "Key id" field
>
>  Set Key Id is a generated item (in KEY and DNSKEY dns types)
>
> Directory: /trunk/epan/dissectors/
>   Changes    Path            Action
>   +6 -4      packet-dns.c    Modified

Just looking at the checkin url made me a bit unhappy. Just look at this:

I'm unhappy too...

[email protected]:~/work/wireshark/svn/trunk/epan/dissectors> perl -ne '/(proto_tree_add_[a-z]+)/ && print "$1\n"' packet-dns.c | sort | uniq -c | sort -nr
   156 proto_tree_add_text
    45 proto_tree_add_item
    21 proto_tree_add_uint
     9 proto_tree_add_string
     3 proto_tree_add_boolean
     2 proto_tree_add_protocol
     1 proto_tree_add_time

So more than half of all the stuff is added by using proto_tree_add_text.
As long as the ratio is that way, people are likely to continue using it
inside this dissector.
Any volunteer(s) to get this down to some sane level by replacing it by
proto_tree_add_item and adding hf_ entries where possible to make these
Elements filterable?

I will see if i have time to update the DNS dissector...

Should something like the above check be added to one of the check scripts
to complain if the add_text percentage is above 10% or so?
 
Good idea ! 

Thanks
  Jörg
--
Joerg Mayer                                           <[email protected]>
We are stuck with technology when what we really want is just stuff that
works. Some say that should read Microsoft instead of technology.
___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <[email protected]>
Archives:    http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
            mailto:[email protected]?subject=unsubscribe