Wireshark-bugs: [Wireshark-bugs] [Bug 6355] add dissectors for tracing SIM <-> Mobile Equipment
Date: Fri, 18 Nov 2011 07:32:08 -0800 (PST)
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=6355 --- Comment #1 from Bill Meier <wmeier@xxxxxxxxxxx> 2011-11-18 10:32:05 EST --- Some comments: packet-gsm_sim.c 1. We've recently changed the usage for the encoding parameter of proto_tree_add_item(). Instead of using TRUE/FALSE we're now using ENC_NA/ENC_BIG_ENDIAN/ENC_LITTLE_ENDIAN depending upon the type. See README.developer (SVN) under proto_tree_add_item() for the rules. It appears to me that the protocol is big-endian so I made a few changes by hand and then used a script (tools/fix-encoding-args.pl) to convert the encoding args as seems appropriate. I've attached a patch file for your use to update your packet-gsm_sim.c source file with the encoding parameter changes.. 2. I'm not really a fan of the use of #if 0/#endif to combine a number of value_strings into one quite large value_string. In addition there are ~35 dups in the combined array. How would you feel about just having one big sorted array with unique entries ? If this is done, then an "extended" value string can be used which (in this case) will do a binary search thru the array. It would be also nice to sort some of the other value_string arrays (such as sw_vals[]) so that they can also accessed via an extended value string. See packet-rsl.c for a recently added example of the use of extended value strings. 3a. There's some #if 0'd code having to do with a preference; in addition the proto_reg_handoff...() code does nothing of any effect. I suggest removing both sections of code. 3b. However, there does need to be a proto_reg_handoff...() with the following (moved from proto_register...()) sub_handle_cap = find_dissector("etsi_cat"); Looking up a dissector shouldn't be done until after all the dissectors have been registered since there's no real guarantee as to the order in which the dissectors are registered. 4. In general, tvb_reported_length() should be used instead of tvb_length(). tvb_reported_length() returns the actual value of the packet length when it was captured while tvb_length() reports the amount of the packet saved. These can differ if the capture was configured to limit the amount of data saved from a packet ("snapshot length"). Wireshark will display an appropriate message (something about "data not available due to snapshot length") if there is an attempt to dissect data not actually available. 5. #include <stdio.h> not needed. packet-card_app_toolkit.c 1. All but one of the proto_tree_add_item() encoding args should be ENC_BIG_ENDIAN. proto_tree_add_item(tree, proto_cat,...) should use ENC_NA. (See item #1 bove). 2. There are several value_string arrays of some length for which it probably makes sense to access them using an extended value string. 3. See item #3a above. 4. See item #4 above. 5. #include <stdio.h> not needed. Other notes 1. epan/CmakeLists.txt also needs to be updated with the new filenames. -- Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
- Prev by Date: [Wireshark-bugs] [Bug 6082] Enhancement of Hilscher Analyzer Dissector
- Next by Date: [Wireshark-bugs] [Bug 6355] add dissectors for tracing SIM <-> Mobile Equipment (3GPP TS 11.11)
- Previous by thread: [Wireshark-bugs] [Bug 5963] Add decryption for resumed TLS sessions with a session ticket
- Next by thread: [Wireshark-bugs] [Bug 6355] add dissectors for tracing SIM <-> Mobile Equipment (3GPP TS 11.11)
- Index(es):
- Get Wireshark
- Download
- Code of Conduct