Wireshark mailing list archives
Re: rev 41952: /trunk/epan/dissectors/ /trunk/epan/dissectors/: packet-ip.c packet-ip.h packet-ipv6.c
From: Anders Broman <a.broman () bredband net>
Date: Wed, 06 Jun 2012 17:17:48 +0200
Joerg Mayer skrev 2012-06-06 16:15:
Ok do as you see fit I'm not going back to figure out what the original problem was.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 () wireshark org 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 ModifiedI 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
Regards Anders ___________________________________________________________________________ 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
Current thread:
- Re: [Wireshark-commits] rev 41952: /trunk/epan/dissectors/ /trunk/epan/dissectors/: packet-ip.c packet-ip.h packet-ipv6.c Joerg Mayer (Jun 05)
- Re: [Wireshark-commits] rev 41952: /trunk/epan/dissectors/ /trunk/epan/dissectors/: packet-ip.c packet-ip.h packet-ipv6.c Anders Broman (Jun 05)
- rev 41952: /trunk/epan/dissectors/ /trunk/epan/dissectors/: packet-ip.c packet-ip.h packet-ipv6.c Joerg Mayer (Jun 06)
- Re: rev 41952: /trunk/epan/dissectors/ /trunk/epan/dissectors/: packet-ip.c packet-ip.h packet-ipv6.c Anders Broman (Jun 06)
- rev 41952: /trunk/epan/dissectors/ /trunk/epan/dissectors/: packet-ip.c packet-ip.h packet-ipv6.c Joerg Mayer (Jun 06)
- Re: [Wireshark-commits] rev 41952: /trunk/epan/dissectors/ /trunk/epan/dissectors/: packet-ip.c packet-ip.h packet-ipv6.c Anders Broman (Jun 05)