Wireshark mailing list archives
Re: Wireshark-commits: [Wireshark-commits] rev 44161: /trunk/epan/dissectors/ /trunk/epan/dissectors/: packet-gmr1_bcch.c packet-gmr1_common.c packet-gmr1_rr.c
From: mmann78 () netscape net
Date: Mon, 20 Aug 2012 00:01:12 -0400 (EDT)
Sylvain, The checkdisplayfilter.pl script reflects my interpretation of the desired display filter format, and since there hasn't been that much feedback on the outputted results (with Pascal's comments on the GSM dissectors being the exception), I continue to plod along manually checking and possibly updating dissectors as they show up in the list. It's not set in stone, but lacking any officially documented rules, I thought it was turning into the defacto standard. I'm more than willing to adapt the script, I just want the consistency expressed in bug 2794. The problem may be defining "consistent". I definitely struggled with the GMR-1 filters (whether they or the script should be changed), and that's one of the main reasons I intentionally made the change its own revision (makes reverting a little easier). I got the impression that the GMR-1 protocols were more a "grouping" of protocols (like the ZigBee or SCSI protocols), than "subdissectors", which is why I went with the underscore separator over the period. I don't see where most users would notice it because you shouldn't see much of a difference in the "autocompletion" when typing in a display filter since the "subdissectors" of common / rr / cc would still be at the top of the list. The CCCH stuff (which appears to be an obvious mistake) came from either another similar dissector architecture, the "protocol filter name" or the naming of the hf_ variables in the registration array (where I found a common theme using their names). The GMR-1 protocol follows one of the biggest reasons for the script - ensuring display filter names start with the "protocol filter name", followed by a period. The problem I have is that I don't like the idea of having a period in the "protocol filter name" itself. This check hasn't been added to the script yet (maybe even to the protocol registration code itself), but I have certainly considered it. To me a period in the "protocol filter name" adds some confusion to what's being "autocompleted" and also suggests that a protocol may have been architected with multiple dissectors to (unnecessarily) break the code up into multiple source modules (for strictly reasons of size). Multiple source modules for a protocol is somewhat discouraged as there are already 1000+ dissector files (with some larger than the totality of the GMR-1 code). If the GMR-1 protocol was implemented in a single dissector/file, the checkdisplayfilter.pl script wouldn't have complained about the "subfilters" of common / rr / cc. -----Original Message----- From: Sylvain Munaut <246tnt () gmail com> To: Developer support list for Wireshark <wireshark-dev () wireshark org> Sent: Sun, Aug 19, 2012 3:17 pm Subject: Re: [Wireshark-dev] Wireshark-commits: [Wireshark-commits] rev 44161: /trunk/epan/dissectors/ /trunk/epan/dissectors/: packet-gmr1_bcch.c packet-gmr1_common.c packet-gmr1_rr.c I don't know all the protocols where this happens but to me having gmr1_rr.xxx or gmr1_common.xxx makes it believe those are different protocols which they're not ... GMR-1 is the protocol and then you have the channel types like CCCH on which you can find messages that can contain elements from RR/CC/... And so having the hierarchy protocol (gmr1) / category of element (common / rr / cc / ...) makes sense to me. Those category are not arbitrary either, they're explicitly defined that way in the official specifications, I'm not making them up. If someone has a good argument _against_ this, I'd be interested to hear it. (And the fact the checkscript can't deal with it is not really a good argument ... at worse we could add meta-tags in the header of those files to define the prefix allowed to be used) Cheers, Sylvain ___________________________________________________________________________ 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
Current thread:
- Re: Wireshark-commits: [Wireshark-commits] rev 44161: /trunk/epan/dissectors/ /trunk/epan/dissectors/: packet-gmr1_bcch.c packet-gmr1_common.c packet-gmr1_rr.c Sylvain Munaut (Aug 19)
- Re: Wireshark-commits: [Wireshark-commits] rev 44161: /trunk/epan/dissectors/ /trunk/epan/dissectors/: packet-gmr1_bcch.c packet-gmr1_common.c packet-gmr1_rr.c Pascal Quantin (Aug 19)
- <Possible follow-ups>
- Re: Wireshark-commits: [Wireshark-commits] rev 44161: /trunk/epan/dissectors/ /trunk/epan/dissectors/: packet-gmr1_bcch.c packet-gmr1_common.c packet-gmr1_rr.c mmann78 (Aug 19)
- Re: Wireshark-commits: [Wireshark-commits] rev 44161: /trunk/epan/dissectors/ /trunk/epan/dissectors/: packet-gmr1_bcch.c packet-gmr1_common.c packet-gmr1_rr.c Pascal Quantin (Aug 21)
- Re: Wireshark-commits: [Wireshark-commits] rev 44161: /trunk/epan/dissectors/ /trunk/epan/dissectors/: packet-gmr1_bcch.c packet-gmr1_common.c packet-gmr1_rr.c mmann78 (Aug 21)
- Re: Wireshark-commits: [Wireshark-commits] rev 44161: /trunk/epan/dissectors/ /trunk/epan/dissectors/: packet-gmr1_bcch.c packet-gmr1_common.c packet-gmr1_rr.c Sylvain Munaut (Aug 22)
- Re: Wireshark-commits: [Wireshark-commits] rev 44161: /trunk/epan/dissectors/ /trunk/epan/dissectors/: packet-gmr1_bcch.c packet-gmr1_common.c packet-gmr1_rr.c Pascal Quantin (Aug 21)