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 <pascal.quantin () gmail com>]

2012/8/20 <mmann78 () netscape net>

>     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.

GMR-1 dissectors, like GSM-A dissectors, can be seen either as separate
protocols belonging to the same family, or as a single protocol with sub
dissectors:it depends a bit on your point of view :) Personnally I see them
as a single one (like Sylvain) and was rather happy with the '.' separator
instead of '_' one.
For example at the beginning in Wireshark there was a single packet-gsm_a.c
file that became huge and was split into several files in r25915 and r25917.
When looking at checkfiltername.pl script, it looks like there are already
variants allowed for PROTOABBREV ('-' vs '_' for example). Would allowing
'.' vs '_' really be an issue? If think it would make sense for big
protocols like GSM or GMR-1 (and there is maybe a few other ones). If
accepted, we should be careful when reviewing patches to ensure that this
capability is not used without any valuable reason.

Regards,
Pascal.

___________________________________________________________________________
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