8 Apr
2016
8 Apr
'16
12:19 a.m.
On 07 Apr 2016, at 13:57, Bhargava Abhyankar
<Bhargava.Abhyankar(a)radisys.com> wrote:
Fix bug in extraction of E and TI header fields in parsing of uplink
header type 1 RLC data block. Test suite for the same is also updated.
- e_ti_header = (data[offs-1] + (data[offs] <<
8)) >> 7;
+ e_ti_header = (data[offs - 1] & (0xc0)) >> 6;
Could you please elaborate about this hunk? What is the issue, how was it found, how are
we certain that this is the fix?
- rlc->block_info[0].e = !!(e_ti_header &
0x01);
- rlc->block_info[0].ti = !!(e_ti_header & 0x02);
+ rlc->block_info[0].e = (e_ti_header & 0x01);
+ rlc->block_info[0].ti = (e_ti_header & 0x02);
what is the issue with this? The intention of !! is to normalize the value to 0 and 1. Now
.ti can have 0x02 and 0x00 as value. Why is that needed?
cur_bit += 2;
rlc->block_info[1].cv = egprs1->cv;
@@ -524,20 +524,21 @@ int Decoding::rlc_parse_ul_data_header_egprs_type_1(
((egprs1->bsn2_a << 0) | (egprs1->bsn2_b << 2));
rlc->block_info[1].bsn = rlc->block_info[1].bsn & (RLC_EGPRS_SNS - 1);
- cur_bit += rlc->data_offs_bits[1] - 2;
+ cur_bit = rlc->data_offs_bits[1] - 2;
Why don't we need to advance this cursor anymore?
offs = rlc->data_offs_bits[1] / 8;
OSMO_ASSERT(rlc->data_offs_bits[1] % 8 == 2);
- e_ti_header = (data[offs-1] + (data[offs] << 8)) >> 7;
+ e_ti_header = (data[offs] & (0x03));
What is the misunderstanding here. How did we think we needed to take bits from two
different octets?
- rlc->block_info[1].e = !!(e_ti_header &
0x01);
- rlc->block_info[1].ti = !!(e_ti_header & 0x02);
+ rlc->block_info[1].e = (e_ti_header & 0x01);
+ rlc->block_info[1].ti = (e_ti_header & 0x02);
Again, why the change from 0 or 1 to .. 0 or 1 .. and 0 or 2?
diff --git a/tests/edge/EdgeTest.cpp
b/tests/edge/EdgeTest.cpp
index ff080f9..9b4a1a5 100644
--- a/tests/edge/EdgeTest.cpp
+++ b/tests/edge/EdgeTest.cpp
@@ -1376,18 +1376,16 @@ static gprs_rlcmac_ul_tbf *uplink_header_parsing_test(BTS
*the_bts,
egprs1->r = 1;
egprs1->cv = 7;
egprs1->tfi_a = tfi & (~((~0) << 2));
- egprs1->tfi_b = tfi & (~((~0) << 3)) << 2;
- egprs1->bsn1_a = 10;
- egprs1->bsn1_b = 17;
- egprs1->bsn2_a = 0;
- egprs1->bsn2_b = 25;
+ egprs1->tfi_b = (tfi & (~((~0) << 3)) << 2) >> 2;
+ egprs1->bsn1_a = 0;
+ egprs1->bsn1_b = 0;
+ egprs1->bsn2_a = 1;
+ egprs1->bsn2_b = 0;
egprs1->cps = 15;
egprs1->rsb = 0;
egprs1->pi = 0;
- data[6] = 1;
- data[6 + 68] = 1;
- data[75] = 1;
- data[75 + 68] = 1;
+ data[5] = 0x40;
+ data[5 + 69] = 1;
Please explain the intention here. What is the issue with tfi_b? Why do you change it. Why
do you pick the new bsn1_a and bsn1_b values?
holger