Wireshark-bugs: [Wireshark-bugs] [Bug 7615] Enhancemnt to GSM RLCMAC dissection adding dissectio
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=7615
Pascal Quantin <pascal.quantin@xxxxxxxxx> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |pascal.quantin@xxxxxxxxx
--- Comment #5 from Pascal Quantin <pascal.quantin@xxxxxxxxx> 2012-08-11 08:04:30 PDT ---
Hi Mike,
here are a few comments after a really quick review:
- were the changes to packet-gsmtap.* approved by Osmocom guys? Please contact
them and get their approval before proposing those changes
- please do not put back Packet_Measurement_Order_Reduced_t structure: it was
removed on purpose
- I guess egprs_data_block_length array should return a guint16 (or other ones
should return a guint8 to keep consistency)
- ensure that filter names match the protocol names: we did an effort to clean
this dissector so let's not introduce non valid filter names now
(gsm_rlcmac.*). It can be checked with the tools/checkfiltername.pl script
- in dissect_gsm_rlcmac_downlink(), data->frame_number does not get initialized
if the private info is not present
- you lack a test to avoid exceeding the maximum number of LI you defined when
filling the array that can lead to a stack corruption
- given their size, you could define dl_rlc_message_type_vals and
ul_rlc_message_type_vals as extended value strings
I'm running out of time right now but I will try to do a more in depth review
in a few days.
--
Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.
