pespin has uploaded this change for review.
tbf_ul: Clean and assert code path generating new UL data block
Clarify the several paths when generating a new BSN (UL data block).
The function create_new_bsn() is only called if there's some LLC frame
ready to be transmitted and put in ul_tbf->llc_tx_msg, so the do while
{} condition checking for no llc_tx_msg is guaranteed to only be run in
the event some LLC frame chunks have been included in the UL RLC block.
This fact allows simplifying the code and at the same time fix a
Coverity CID regarding a code path which could not really happen. We now
guard this case with an ASSERT.
Related: Coverity CID#345063
Change-Id: I6d4ae377a60839f76295c0e7e6aaa1fe53e37315
---
M src/rlcmac/tbf_ul.c
1 file changed, 37 insertions(+), 15 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmo-gprs refs/changes/82/35882/1
diff --git a/src/rlcmac/tbf_ul.c b/src/rlcmac/tbf_ul.c
index 0ea0a9c..67c9e98 100644
--- a/src/rlcmac/tbf_ul.c
+++ b/src/rlcmac/tbf_ul.c
@@ -684,6 +684,7 @@
if (!ul_tbf->llc_tx_msg || msgb_length(ul_tbf->llc_tx_msg) == 0)
gprs_rlcmac_ul_tbf_schedule_next_llc_frame(ul_tbf);
+ /* This function shall only be called if there's some LLC payload not yet transmitted: */
OSMO_ASSERT(ul_tbf->llc_tx_msg);
OSMO_ASSERT(gprs_rlcmac_mcs_is_valid(cs));
@@ -734,27 +735,29 @@
int payload_written = 0;
if (!ul_tbf->llc_tx_msg || msgb_length(ul_tbf->llc_tx_msg) == 0) {
+ const int space = block_data_len - write_offset;
+ /* Only get here in case we already encoded the end of
+ * some LLC payload chunk and there's no more new LLC
+ * payloads to send */
+ OSMO_ASSERT(num_chunks > 0);
+
/* The data just drained, store the current fn */
if (ul_tbf->last_ul_drained_fn < 0)
ul_tbf->last_ul_drained_fn = bi->fn;
- int space = block_data_len - write_offset;
+ /* Nothing to send, and we already put some data in
+ * rlcmac data block, we are done */
+ LOGPTBFUL(ul_tbf, LOGL_DEBUG,
+ "LLC queue completely drained and there's "
+ "still %d free bytes in rlcmac data block\n", space);
- if (num_chunks != 0) {
- /* Nothing to send, and we already put some data in
- * rlcmac data block, we are done */
- LOGPTBFUL(ul_tbf, LOGL_DEBUG,
- "LLC queue completely drained and there's "
- "still %d free bytes in rlcmac data block\n", space);
-
- if (gprs_rlcmac_mcs_is_edge(cs)) {
- /* in EGPRS there's no M bit, so we need
- * to flag padding with LI=127 */
- gprs_rlcmac_rlc_data_to_ul_append_egprs_li_padding(
- rdbi, &write_offset, &num_chunks, data);
- }
- break;
+ if (gprs_rlcmac_mcs_is_edge(cs)) {
+ /* in EGPRS there's no M bit, so we need to flag
+ * padding with LI=127 */
+ gprs_rlcmac_rlc_data_to_ul_append_egprs_li_padding(
+ rdbi, &write_offset, &num_chunks, data);
}
+ break;
}
ar = gprs_rlcmac_enc_append_ul_data(rdbi, cs, ul_tbf->llc_tx_msg, &write_offset,
To view, visit change 35882. To unsubscribe, or for help writing mail filters, visit settings.