Huge thanks to our Platinum Members Endace and LiveAction,
and our Silver Member Veeam, for supporting the Wireshark Foundation and project.

Wireshark-bugs: [Wireshark-bugs] [Bug 7615] Enhancemnt to GSM RLCMAC dissection adding dissectio

Date: Sat, 11 Aug 2012 08:04:31 -0700 (PDT)
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.