[PATCH] FANR UL and DL flow changes and UT fixes

[Holger Freyther]

> On 10 Mar 2016, at 06:45, Aravind Sirsikar <Arvind.Sirsikar at radisys.com> wrote:


Hi,

the main issue is already in the subject of the patch. "Do this ... and ... that" is better done as two separate patches.

Let me just give you one example



> -int Decoding::rlc_parse_ul_data_header(struct gprs_rlc_ul_header_generic *rlc,
> -	const uint8_t *data, GprsCodingScheme cs)
> +int Decoding::decode_ul_data_fanr_type1(struct gprs_rlc_ul_header_generic *rlc,
> +		const uint8_t *data, GprsCodingScheme* cs)
> {
> -	const struct rlc_ul_header *gprs;
> 	unsigned int data_len = 0;
> +	data_len = cs->maxDataBlockBytes();
> +
> +	const struct gprs_rlc_ul_header_egprs_fanr_type1 *egprs1;
> +	egprs1 = static_cast<struct gprs_rlc_ul_header_egprs_fanr_type1*>
> +		((void *)data);
> +	rlc->r      = egprs1->r;
> +	rlc->si     = egprs1->si;
> +	rlc->tfi    = (egprs1->tfi_a << 0)  | (egprs1->tfi_b << 2);
> +	rlc->cps    = egprs1->cps;
> +	rlc->rsb    = egprs1->rsb;
> +	rlc->cv     = egprs1->cv;
> +	rlc->pani     = egprs1->pani;
> +
> +	rlc->num_data_blocks = 2;
> +	rlc->block_info[0].cv  = egprs1->cv;
> +	rlc->block_info[0].pi  = egprs1->pi;
> +	rlc->block_info[0].bsn =
> +		(egprs1->bsn1_a << 0) | (egprs1->bsn1_b << 5);
> +
> +	rlc->block_info[0].spb = 0;
> +
> +	rlc->block_info[1].cv  = egprs1->cv;
> +	rlc->block_info[1].pi  = egprs1->pi;
> +
> +	rlc->block_info[1].bsn = rlc->block_info[0].bsn + ((egprs1->bsn2_a << 0) | (egprs1->bsn2_b << 2));
> +	rlc->block_info[1].bsn = (rlc->block_info[1].bsn + 2048) % 2047;
> +
> +	rlc->block_info[0].e   = (data[6] & 0x01);
> +	rlc->block_info[0].ti  = (data[6] &  0x02) >> 1;
> +	rlc->block_info[1].e   = (data[6 + data_len + 1] & 0x01);
> +	rlc->block_info[1].ti  = (data[6 + data_len + 1] &  0x02) >> 1;
> +	rlc->block_info[1].spb = 0;
> +
> +	rlc->block_info[0].data_len = data_len;
> +	rlc->block_info[1].data_len = data_len;
> +	rlc->data_offs_bytes[0] = 7;
> +	rlc->data_offs_bytes[1] = 7 + data_len + 1;
> +	return 0;
> +}
> +




> +int Decoding::rlc_parse_ul_data_header(struct gprs_rlc_ul_header_generic *rlc,
> +	const uint8_t *data, GprsCodingScheme cs)


> -		rlc->pani= 0;
> +		rlc->pani   = 0;

please don't do drive-by changes.



> -	case GprsCodingScheme::HEADER_EGPRS_DATA_TYPE_3:
> -		const struct gprs_rlc_ul_header_egprs_3 *egprs3;
> -		egprs3 = static_cast<struct gprs_rlc_ul_header_egprs_3 *>
> -                        ((void *)data);
> -                rlc->r      = egprs3->r;
> -                rlc->si     = egprs3->si;
> -                rlc->tfi    = (egprs3->tfi_a << 0)  | (egprs3->tfi_b << 2);
> -                rlc->cps    = (egprs3->cps_a << 0)  | (egprs3->cps_b << 2);
> -                rlc->rsb    = egprs3->rsb;
> -                rlc->cv     = egprs3->cv;
> -                /* pani 0 for fanr inactive */
> -		rlc->pani     = 0;
> -
> -                rlc->num_data_blocks = 1;
> -                rlc->block_info[0].cv  = egprs3->cv;
> -                rlc->block_info[0].pi  = egprs3->pi;
> -                rlc->block_info[0].spb = egprs3->spb;
> -                rlc->block_info[0].bsn =
> -                        (egprs3->bsn1_a << 0) | (egprs3->bsn1_b << 5);
> -
> -                rlc->block_info[0].e   = (data[4] & 0x01);
> -                rlc->block_info[0].ti  = (data[4] &  0x02) >> 1;
> -
> -                rlc->block_info[0].data_len = data_len;
> -                rlc->data_offs_bytes[0] = 5;
> -
> -		if(rlc->block_info[0].spb == 2){
> -			if(cs == GprsCodingScheme::MCS3){ 
> -				if(rlc->cps == 6 || rlc->cps == 7 || rlc->cps == 8){
> -        	        		rlc->block_info[0].data_len -= pad_bytes;
> -               				rlc->data_offs_bytes[0] += pad_bytes;
> -                			rlc->block_info[0].e   = (data[4 + pad_bytes] & 0x01);
> -			                rlc->block_info[0].ti  = (data[4 + pad_bytes] &  0x02) >> 1;
> -				}
> -			}
> -		}
> -		break;



Okay, it is not the same method that was moved but as a rule of thumb. if you move code, first copy the code in a commit and then modify it. This makes the actual change visible.

With a patch series you tell a story and need to leave the code in a better state than it was before and additional features. To do this we have small commits.


holger