csn1: fix: never use enumerated types in codec structures
I faced a problem while working on EGPRS Packet Channel Request coding support: the unit test I wrote for it was passing when compiled with AddressSanitizer, but failing when compiled without it o_O. Somehow this was observed only with GCC 10. Here is a part the standard output diff for that unit test: *** testEGPRSPktChReq *** decode_egprs_pkt_ch_req(0x2b5) returns 0 - ==> One Phase Access + ==> unknown 0xdd5f4e00 decode_egprs_pkt_ch_req(0x14a) returns 0 - ==> One Phase Access + ==> unknown 0xdd5f4e00 decode_egprs_pkt_ch_req(0x428) returns 0 - ==> Short Access + ==> unknown 0xdd5f4e01 At the same time, debug output of the CSN.1 decoder looked fine. So WYSINWYG (What You See Is *NOT* What You Get)! As it turned out, this was happening because I used an enumerated type to represent the sub-type of EGPRS Packet Channel Request. typedef struct { EGPRS_PacketChannelRequestType_t Type; // <-- enum EGPRS_PacketChannelRequestContent_t Content; } EGPRS_PacketChannelRequest_t; The problem is that length of an enumerated field, more precisely the amount of bytes it takes in the memory, is compiler/machine dependent. While the CSN.1 decoder assumes that the field holding sequential number of the chosen element is one octet long, so its address is getting casted to (guint8 *) and the value is written to the first MSB. // csnStreamDecoder(), case CSN_CHOICE: pui8 = pui8DATA(data, pDescr->offset); *pui8 = i; // [ --> xx .. .. .. ] Let's make sure that none of the existing RLC/MAC definitions is using enumerated types, and add a warning comment to CSN_CHOICE. Affected CSN.1 definitions (unit test output adjusted): - Additional_access_technologies_struct_t, - Channel_Request_Description_t. Change-Id: I917a40647480c6f6f3b0e68674ce9894379a9e7f
This commit is contained in:
parent
0614e9333f
commit
93ad3fd9b9
|
@ -493,9 +493,11 @@ gint16 csnStreamEncoder(csnStream_t* ar, const CSN_DESCR* pDescr, struct bitvec
|
|||
* The value of the address is called a selector. Up to 256 (UCHAR_MAX) unique
|
||||
* selectors can be handled, longer choice list would cause CSN_ERROR_IN_SCRIPT.
|
||||
* After unpacking, this value is then converted to the sequential number of the
|
||||
* element in the union and stored in the UnionType variable.
|
||||
* element in the union and stored in the UnionType variable (Par2).
|
||||
* Par1: C structure name
|
||||
* Par2: C structure element name
|
||||
* Par2: C structure field name holding sequential number of the chosen element.
|
||||
* BEWARE! Never use an enumerated type here, because its length is
|
||||
* compiler/machine dependent, while decoder would cast it to guint8.
|
||||
* Par3: address of an array of type CSN_ChoiceElement_t where all possible
|
||||
* values of the selector are provided, together with the selector
|
||||
* length expressed in bits and the address of the CSN_DESCR type
|
||||
|
|
|
@ -158,7 +158,7 @@ typedef struct
|
|||
{
|
||||
guint8 PEAK_THROUGHPUT_CLASS;
|
||||
guint8 RADIO_PRIORITY;
|
||||
RLC_MODE_t RLC_MODE;
|
||||
guint8 RLC_MODE;
|
||||
guint8 LLC_PDU_TYPE;
|
||||
guint16 RLC_OCTET_COUNT;
|
||||
} Channel_Request_Description_t;
|
||||
|
@ -1245,7 +1245,7 @@ typedef enum
|
|||
|
||||
typedef struct
|
||||
{
|
||||
AccessTechnology_t Access_Technology_Type;
|
||||
guint8 Access_Technology_Type;
|
||||
guint8 GMSK_Power_class;
|
||||
guint8 Eight_PSK_Power_class;
|
||||
} Additional_access_technologies_struct_t;
|
||||
|
|
File diff suppressed because one or more lines are too long
Loading…
Reference in New Issue