cosmetic: clarify coding scheme and puncturing

* use appropriate types for coding scheme parameters
* add comment regarding possible number of RLCMAC blocks

The code in create_dl_acked_block() has underlying assumption that
rlc.num_data_blocks can never be more than 2, which is true and is
enforced by appropriate asserts but is not obvious when looking at the
function code alone. It's equally hard for Coverity which leads to false
positives in scan.

Lets' make this assumption explicit by putting it into for(;;) condition
alongside with corresponding comment.

Fixes: CID143070
Change-Id: If599a6c8a6ef56d847604fcf41bb71decccd8a78

5 files changed, 45 insertions, 45 deletions

diff --git a/src/gprs_coding_scheme.cpp b/src/gprs_coding_scheme.cpp
index 719cbb28..3094ae6c 100644
--- a/src/gprs_coding_scheme.cpp
+++ b/src/gprs_coding_scheme.cpp
@@ -55,12 +55,12 @@ enum GprsCodingScheme::Scheme GprsCodingScheme::egprs_mcs_retx_tbl[MAX_NUM_ARQ]
 
 static struct {
 	struct {
-		unsigned int bytes;
-		unsigned int ext_bits;
-		unsigned int data_header_bits;
+		uint8_t bytes;
+		uint8_t ext_bits;
+		uint8_t data_header_bits;
 	} uplink, downlink;
-	unsigned int data_bytes;
-	unsigned int optional_padding_bits;
+	uint8_t data_bytes;
+	uint8_t optional_padding_bits;
 	const char *name;
 	GprsCodingScheme::HeaderType data_hdr;
 	GprsCodingScheme::Family family;
@@ -99,10 +99,10 @@ static struct {
 
 static struct {
 	struct {
-		int data_header_bits;
+		uint8_t data_header_bits;
 	} uplink, downlink;
-	unsigned int data_block_header_bits;
-	unsigned int num_blocks;
+	uint8_t data_block_header_bits;
+	uint8_t num_blocks;
 	const char *name;
 } hdr_type_info[GprsCodingScheme::NUM_HEADER_TYPES] = {
 	{{0},       {0},       0, 0, "INVALID"},
@@ -134,12 +134,12 @@ GprsCodingScheme GprsCodingScheme::getBySizeUL(unsigned size)
 	return GprsCodingScheme(UNKNOWN);
 }
 
-unsigned int GprsCodingScheme::sizeUL() const
+uint8_t GprsCodingScheme::sizeUL() const
 {
 	return mcs_info[m_scheme].uplink.bytes + (spareBitsUL() ? 1 : 0);
 }
 
-unsigned int GprsCodingScheme::usedSizeUL() const
+uint8_t GprsCodingScheme::usedSizeUL() const
 {
 	if (mcs_info[m_scheme].data_hdr == HEADER_GPRS_DATA)
 		return mcs_info[m_scheme].uplink.bytes;
@@ -147,22 +147,22 @@ unsigned int GprsCodingScheme::usedSizeUL() const
 		return sizeUL();
 }
 
-unsigned int GprsCodingScheme::maxBytesUL() const
+uint8_t GprsCodingScheme::maxBytesUL() const
 {
 	return mcs_info[m_scheme].uplink.bytes;
 }
 
-unsigned int GprsCodingScheme::spareBitsUL() const
+uint8_t GprsCodingScheme::spareBitsUL() const
 {
 	return mcs_info[m_scheme].uplink.ext_bits;
 }
 
-unsigned int GprsCodingScheme::sizeDL() const
+uint8_t GprsCodingScheme::sizeDL() const
 {
 	return mcs_info[m_scheme].downlink.bytes + (spareBitsDL() ? 1 : 0);
 }
 
-unsigned int GprsCodingScheme::usedSizeDL() const
+uint8_t GprsCodingScheme::usedSizeDL() const
 {
 	if (mcs_info[m_scheme].data_hdr == HEADER_GPRS_DATA)
 		return mcs_info[m_scheme].downlink.bytes;
@@ -170,42 +170,42 @@ unsigned int GprsCodingScheme::usedSizeDL() const
 		return sizeDL();
 }
 
-unsigned int GprsCodingScheme::maxBytesDL() const
+uint8_t GprsCodingScheme::maxBytesDL() const
 {
 	return mcs_info[m_scheme].downlink.bytes;
 }
 
-unsigned int GprsCodingScheme::spareBitsDL() const
+uint8_t GprsCodingScheme::spareBitsDL() const
 {
 	return mcs_info[m_scheme].downlink.ext_bits;
 }
 
-unsigned int GprsCodingScheme::maxDataBlockBytes() const
+uint8_t GprsCodingScheme::maxDataBlockBytes() const
 {
 	return mcs_info[m_scheme].data_bytes;
 }
 
-unsigned int GprsCodingScheme::optionalPaddingBits() const
+uint8_t GprsCodingScheme::optionalPaddingBits() const
 {
 	return mcs_info[m_scheme].optional_padding_bits;
 }
 
-unsigned int GprsCodingScheme::numDataBlocks() const
+uint8_t GprsCodingScheme::numDataBlocks() const
 {
 	return hdr_type_info[headerTypeData()].num_blocks;
 }
 
-unsigned int GprsCodingScheme::numDataHeaderBitsUL() const
+uint8_t GprsCodingScheme::numDataHeaderBitsUL() const
 {
 	return hdr_type_info[headerTypeData()].uplink.data_header_bits;
 }
 
-unsigned int GprsCodingScheme::numDataHeaderBitsDL() const
+uint8_t GprsCodingScheme::numDataHeaderBitsDL() const
 {
 	return hdr_type_info[headerTypeData()].downlink.data_header_bits;
 }
 
-unsigned int GprsCodingScheme::numDataBlockHeaderBits() const
+uint8_t GprsCodingScheme::numDataBlockHeaderBits() const
 {
 	return hdr_type_info[headerTypeData()].data_block_header_bits;
 }
diff --git a/src/gprs_coding_scheme.h b/src/gprs_coding_scheme.h
index dfad240b..76cab0f2 100644
--- a/src/gprs_coding_scheme.h
+++ b/src/gprs_coding_scheme.h
@@ -70,7 +70,7 @@ public:
 
 	operator bool() const {return m_scheme != UNKNOWN;}
 	operator Scheme() const {return m_scheme;}
-	unsigned int to_num() const;
+	uint8_t to_num() const;
 
 	GprsCodingScheme& operator =(Scheme s);
 	bool operator == (Scheme s) const;
@@ -91,20 +91,20 @@ public:
 	void dec();
 	void decToSingleBlock(bool *needStuffing);
 
-	unsigned int sizeUL() const;
-	unsigned int sizeDL() const;
-	unsigned int usedSizeUL() const;
-	unsigned int usedSizeDL() const;
-	unsigned int maxBytesUL() const;
-	unsigned int maxBytesDL() const;
-	unsigned int spareBitsUL() const;
-	unsigned int spareBitsDL() const;
-	unsigned int maxDataBlockBytes() const;
-	unsigned int numDataBlocks() const;
-	unsigned int numDataHeaderBitsUL() const;
-	unsigned int numDataHeaderBitsDL() const;
-	unsigned int numDataBlockHeaderBits() const;
-	unsigned int optionalPaddingBits() const;
+	uint8_t sizeUL() const;
+	uint8_t sizeDL() const;
+	uint8_t usedSizeUL() const;
+	uint8_t usedSizeDL() const;
+	uint8_t maxBytesUL() const;
+	uint8_t maxBytesDL() const;
+	uint8_t spareBitsUL() const;
+	uint8_t spareBitsDL() const;
+	uint8_t maxDataBlockBytes() const;
+	uint8_t numDataBlocks() const;
+	uint8_t numDataHeaderBitsUL() const;
+	uint8_t numDataHeaderBitsDL() const;
+	uint8_t numDataBlockHeaderBits() const;
+	uint8_t optionalPaddingBits() const;
 	const char *name() const;
 	HeaderType headerTypeData() const;
 	HeaderType headerTypeControl() const;
@@ -127,7 +127,7 @@ private:
 	enum Scheme m_scheme;
 };
 
-inline unsigned int GprsCodingScheme::to_num() const
+inline uint8_t GprsCodingScheme::to_num() const
 {
 	if (isGprs())
 		return (m_scheme - CS1) + 1;
diff --git a/src/rlc.cpp b/src/rlc.cpp
index 37e83cda..d7f06090 100644
--- a/src/rlc.cpp
+++ b/src/rlc.cpp
@@ -375,7 +375,7 @@ void gprs_rlc_data_block_info_init(struct gprs_rlc_data_block_info *rdbi,
 
 unsigned int gprs_rlc_mcs_cps(GprsCodingScheme cs,
 	enum egprs_puncturing_values punct,
-	enum egprs_puncturing_values punct2, int with_padding)
+	enum egprs_puncturing_values punct2, bool with_padding)
 {
 	/* validate that punct and punct2 are as expected */
 	switch (GprsCodingScheme::Scheme(cs)) {
diff --git a/src/rlc.h b/src/rlc.h
index b62e3ac5..a7e6cf34 100644
--- a/src/rlc.h
+++ b/src/rlc.h
@@ -163,7 +163,7 @@ struct gprs_rlc_data_info {
 	unsigned int es_p;
 	unsigned int rrbp;
 	unsigned int pr;
-	unsigned int num_data_blocks;
+	uint8_t num_data_blocks; /* this can actually be only 0, 1, 2: enforced in gprs_rlc_data_header_init() */
 	unsigned int with_padding;
 	unsigned int data_offs_bits[2];
 	struct gprs_rlc_data_block_info block_info[2];
@@ -218,7 +218,7 @@ void gprs_rlc_data_info_init_ul(struct gprs_rlc_data_info *rlc,
 void gprs_rlc_data_block_info_init(struct gprs_rlc_data_block_info *rdbi,
 	GprsCodingScheme cs, bool with_padding, const unsigned int spb);
 unsigned int gprs_rlc_mcs_cps(GprsCodingScheme cs, enum egprs_puncturing_values
-	punct, enum egprs_puncturing_values punct2, int with_padding);
+	punct, enum egprs_puncturing_values punct2, bool with_padding);
 void gprs_rlc_mcs_cps_decode(unsigned int cps, GprsCodingScheme cs,
 	int *punct, int *punct2, int *with_padding);
 enum egprs_puncturing_values gprs_get_punct_scheme(enum egprs_puncturing_values
diff --git a/src/tbf_dl.cpp b/src/tbf_dl.cpp
index 58b55dcd..b871bc32 100644
--- a/src/tbf_dl.cpp
+++ b/src/tbf_dl.cpp
@@ -666,7 +666,7 @@ struct msgb *gprs_rlcmac_dl_tbf::create_dl_acked_block(
 	unsigned msg_len;
 	bool need_poll;
 	/* TODO: support MCS-7 - MCS-9, where data_block_idx can be 1 */
-	unsigned int data_block_idx = 0;
+	uint8_t data_block_idx = 0;
 	unsigned int rrbp;
 	uint32_t new_poll_fn;
 	int rc;
@@ -752,8 +752,8 @@ struct msgb *gprs_rlcmac_dl_tbf::create_dl_acked_block(
 
 	LOGP(DRLCMACDL, LOGL_DEBUG, "- Copying %u RLC blocks, %u BSNs\n", rlc.num_data_blocks, num_bsns);
 
-	/* Copy block(s) to RLC message */
-	for (data_block_idx = 0; data_block_idx < rlc.num_data_blocks;
+	/* Copy block(s) to RLC message: the num_data_blocks cannot be more than 2 - see assert above */
+	for (data_block_idx = 0; data_block_idx < OSMO_MIN(rlc.num_data_blocks, 2);
 		data_block_idx++)
 	{
 		int bsn;
@@ -777,7 +777,7 @@ struct msgb *gprs_rlcmac_dl_tbf::create_dl_acked_block(
 			OSMO_ASSERT(m_rlc.block(bsn)->next_ps >= EGPRS_PS_1);
 			OSMO_ASSERT(m_rlc.block(bsn)->next_ps <= EGPRS_PS_3);
 		}
-		OSMO_ASSERT(data_block_idx < 2); /* punct defined above as 2-element array */
+
 		punct[data_block_idx] = m_rlc.block(bsn)->next_ps;
 
 		rdbi = &rlc.block_info[data_block_idx];