Wireshark · Wireshark-dev: Re: [Wireshark-dev] rev 41952: /trunk/epan/dissectors/ /trunk/epan/dissectors/: packet-ip.c packet-i

[Anders Broman <a.broman@xxxxxxxxxxxx>]

Joerg Mayer skrev 2012-06-06 16:15:
> On Wed, Jun 06, 2012 at 08:46:37AM +0200, Anders Broman wrote:
> > Joerg Mayer skrev 2012-06-05 21:52:
> > > On Thu, Apr 05, 2012 at 08:38:26AM +0000, etxrab@xxxxxxxxxxxxx wrote:
> > > > http://anonsvn.wireshark.org/viewvc/viewvc.cgi?view=rev&revision=41952
> > > >
> > > > User: etxrab
> > > > Date: 2012/04/05 01:38 AM
> > > >
> > > > Log:
> > > >   Use common code to add ip version to the tree.
> > > >
> > > > Directory: /trunk/epan/dissectors/
> > > >    Changes    Path             Action
> > > >    +8 -4      packet-ip.c      Modified
> > > >    +2 -0      packet-ip.h      Modified
> > > >    +1 -18     packet-ipv6.c    Modified
> > > I strongly disagree with this patch and the patch doing the same to dscp
> > > (and the corresponding bug):
> > >
> > > It completely violates the principle of least surprise: Previous to this
> > > patch *all* filterable elements of the ipv6 protocol had filters starting
> > > with ipv6. - this is no longer the case. The reason we have the ip_version
> > > in ipv6 *in addition to* ipv6_version was exactly this: Some people might
> > > be surprised if ip.version==6 would not work - principle of least surprise
> > > again.
> > >
> > > I'm very much for reverting r41952 and r41953 - so much so that I will do
> > > exactly this if there are no strong arguments to not do this.
> > If I remember correctly the user complaining about this was
> > surprised that clicking on ip.version in an IPv4
> > packet and doing prepare as a filter and changing the value to 6 got
> > no IPv6 packets match in a mixed Ipv4 Ipv6
> > trace.
> This is very weird, bacause the ipv6_version field could be filtered with
> ip.version and ipv6.version before the patch. Looks to me that something
> else was wrong.
>
> > I fail to see the importance of every field name without
> > exception adhering to the rule of being prefixed
> > with "protoname". To me this is a legitimate exception.
> Well, if we do an exception there should be good reasons for this and it
> should be documented in the source. I fail to see good reasons in both
> cases. I reopened the dscp bug to see whether the reporter and patch author
> has some good reasons - the already given ones are not IMO - breaking
> consistency for convenience in some "corner cases" rarely is good design.
>
> ciao
>       Jörg
Ok do as you see fit I'm not going back to figure out what the original 
problem was.
Regards
Anders