Re: [Wireshark-commits] rev 40877: /trunk/epan/dissectors/ /trunk/epan/dissectors/: packet-dns.c

[Alexis La Goutte <alexis.lagoutte () gmail com>]

Hi,

On Tue, Feb 7, 2012 at 8:20 AM, Joerg Mayer <jmayer () loplof de> wrote:

> On Mon, Feb 06, 2012 at 04:36:23PM +0000, alagoutte () wireshark org 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...

> jmayer@egg:~/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                                           <jmayer () loplof de>
> 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 <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
___________________________________________________________________________
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