To quote the comment in plugins/epan/gryphon/packet-gryphon.c, "Note:
using external tfs strings doesn't work in a plugin", so, for plugins,
don't check to make sure that the file doesn't define a
true_false_string that's the same as one defined by tfs.c.
For CI, will now return error codes only for those issues
that are definitely bugs that will require fixing. i.e.
- if the type is not compatible with the call
- if a TFS is (case-sensitively) identical to a tfs.c entrywq
Add a check to point out where consecutive items have the same filter
but different labels. Quite a few of these look like bugs.
Also, make some REs raw strings, as identified as an issue in
https://gitlab.com/wireshark/wireshark/-/merge_requests/346
Fix some issues discovered by common python linters including:
* switch `None` comparisons to use `is` rather than `==`. Identity !=
equality, and I've spent 40+ hours before tracking down a subtle bug
caused by exactly this issue. Note that this may introduce a problem if
one of the scripts is depending on this behavior, in which case the
comparison should be changed to `True`/`False` rather than `None`.
* Use `except Exception:` as bare `except:` statements have been
discouraged for years. Ideally for some of these we'd examine if there
were specific exceptions that should be caught, but for now I simply
caught all. Again, this could introduce very subtle behavioral changes
under Python 2, but IIUC, that was all fixed in Python 3, so safe to
move to `except Exception:`.
* Use more idiomatic `if not x in y`--> `if x not in y`
* Use more idiomatic 2 blank lines. I only did this at the beginning,
until I realized how overwhelming this was going to be to apply, then I
stopped.
* Add a TODO where an undefined function name is called, so will fail
whenever that code is run.
* Add more idiomatic spacing around `:`. This is also only partially
cleaned up, as I gave up when I saw how `asn2wrs.py` was clearly
infatuated with the construct.
* Various other small cleanups, removed some trailing whitespace and
improper indentation that wasn't a multiple of 4, etc.
There is still _much_ to do, but I haven't been heavily involved with
this project before, so thought this was a sufficient amount to put up
and see what the feedback is.
Linters that I have enabled which highlighted some of these issues
include:
* `pylint`
* `flake8`
* `pycodestyle`
'check_tfs.py --common' can look for tfs values that appear multiple times.
Current output prior to these dssector changes was:
('No Extension', 'Extension') appears 3 times in: ['epan/dissectors/packet-bssap.c', 'epan/dissectors/packet-camel.c', 'epan/dissectors/packet-gsm_map.c']
('Optimised for signalling traffic', 'Not optimised for signalling traffic') appears 3 times in: ['epan/dissectors/packet-gsm_a_gm.c', 'epan/dissectors/packet-gsm_map.c', 'epan/dissectors/packet-gtp.c']
('Data PDU', 'Control PDU') appears 3 times in: ['epan/dissectors/packet-pdcp-lte.c', 'epan/dissectors/packet-pdcp-nr.c', 'epan/dissectors/packet-rlc-nr.c']
('Message sent to originating side', 'Message sent from originating side') appears 3 times in: ['epan/dissectors/packet-q2931.c', 'epan/dissectors/packet-q931.c', 'epan/dissectors/packet-q933.c']
('User', 'Provider') appears 3 times in: ['epan/dissectors/packet-q2931.c', 'epan/dissectors/packet-q931.c', 'epan/dissectors/packet-q933.c']
The first and last ones were made common, the others seem a little too specialised.
Checking some of the existing items in tfs.c (using QtCreator's 'Find Usages'),
some of the common items are used a lot, but many of them are not referenced.
Change-Id: Ia4006d2c4fa7cafbc3b004dc7a367a986dbeb0c4
Reviewed-on: https://code.wireshark.org/review/38177
Petri-Dish: Martin Mathieson <martin.r.mathieson@googlemail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
Found using tools/check_tfs.py, included in this commit.
Here are the reports that were fixed here:
Examining:
All dissector modules
epan/dissectors/packet-assa_r3.c tfs_mortisepins_flags - could have used tfs_high_low from tfs.c instead: {High,Low}
epan/dissectors/packet-btle.c tfs_present_bit - could have used tfs_present_not_present from tfs.c instead: {Present,Not Present}
epan/dissectors/packet-dhcp.c tfs_fqdn_s - could have used tfs_server_client from tfs.c instead: {Server,Client}
epan/dissectors/packet-docsis-macmgmt.c mdd_tfs_on_off - could have used tfs_on_off from tfs.c instead: {On,Off}
epan/dissectors/packet-docsis-macmgmt.c mdd_tfs_en_dis - could have used tfs_enabled_disabled from tfs.c instead: {Enabled,Disabled}
epan/dissectors/packet-docsis-macmgmt.c req_not_req_tfs - could have used tfs_requested_not_requested from tfs.c instead: {Requested,Not Requested}
epan/dissectors/packet-docsis-tlv.c on_off_tfs - could have used tfs_on_off from tfs.c instead: {On,Off}
epan/dissectors/packet-docsis-tlv.c activation_tfs - could have used tfs_active_inactive from tfs.c instead: {Active,Inactive}
epan/dissectors/packet-docsis.c ena_dis_tfs - could have used tfs_enabled_disabled from tfs.c instead: {Enabled,Disabled}
epan/dissectors/packet-ecmp.c tfs_not_expected_expected - could have used tfs_odd_even from tfs.c instead: {Odd,Even}
epan/dissectors/packet-erf.c erf_link_status_tfs - could have used tfs_up_down from tfs.c instead: {Up,Down}
epan/dissectors/packet-h263.c on_off_flg - could have used tfs_on_off from tfs.c instead: {On,Off}
epan/dissectors/packet-h263.c cpm_flg - could have used tfs_on_off from tfs.c instead: {On,Off}
epan/dissectors/packet-interlink.c flags_set_notset - could have used tfs_set_notset from tfs.c instead: {Set,Not set}
epan/dissectors/packet-ip.c tos_set_low - could have used tfs_low_normal from tfs.c instead: {Low,Normal}
epan/dissectors/packet-ip.c tos_set_high - could have used tfs_high_normal from tfs.c instead: {High,Normal}
epan/dissectors/packet-isakmp.c flag_r - could have used tfs_response_request from tfs.c instead: {Response,Request}
epan/dissectors/packet-isis-lsp.c tfs_metric_supported_not_supported - could have used tfs_no_yes from tfs.c instead: {No,Yes}
epan/dissectors/packet-kerberos.c supported_tfs - could have used tfs_supported_not_supported from tfs.c instead: {Supported,Not supported}
epan/dissectors/packet-kerberos.c set_tfs - could have used tfs_set_notset from tfs.c instead: {Set,Not set}
epan/dissectors/packet-mac-lte.c mac_lte_scell_status_vals - could have used tfs_activated_deactivated from tfs.c instead: {Activated,Deactivated}
epan/dissectors/packet-p_mul.c no_yes - could have used tfs_no_yes from tfs.c instead: {No,Yes}
epan/dissectors/packet-pgm.c opts_present - could have used tfs_present_not_present from tfs.c instead: {Present,Not Present}
epan/dissectors/packet-rsl.c rsl_ms_fpc_epc_mode_vals - could have used tfs_inuse_not_inuse from tfs.c instead: {In use,Not in use}
epan/dissectors/packet-sita.c tfs_sita_on_off - could have used tfs_on_off from tfs.c instead: {On,Off}
epan/dissectors/packet-vines.c tfs_vine_rtp_no_yes - could have used tfs_no_yes from tfs.c instead: {No,Yes}
epan/dissectors/packet-vnc.c button_mask_tfs - could have used tfs_pressed_not_pressed from tfs.c instead: {Pressed,Not pressed}
27 issues found
Change-Id: I7e53b491f20289955c9e9caa8357197d9010a5aa
Reviewed-on: https://code.wireshark.org/review/38087
Petri-Dish: Martin Mathieson <martin.r.mathieson@googlemail.com>
Reviewed-by: Anders Broman <a.broman58@gmail.com>
Martin Mathieson