Change in osmo-pcu[master]: csn1: fix csnStreamDecoder(): avoid conditional calls to bitvec_read_...

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

laforge gerrit-no-reply at lists.osmocom.org
Thu Feb 6 16:19:47 UTC 2020


laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/17091 )

Change subject: csn1: fix csnStreamDecoder(): avoid conditional calls to bitvec_read_field()
......................................................................

csn1: fix csnStreamDecoder(): avoid conditional calls to bitvec_read_field()

As was discovered by pespin, changing logging level of DCSN1 makes
the CSN.1 decoder behave differently (see OS#4375). In particular,
this makes RLCMACTest (encode / decode test) fail.

I did a quick investigation and noticed that some of the logging
statements call bitvec_read_field(). By definition this function
moves the internal pointer (current bit position) of a given
vector and increments readIndex by a given amount of bits.

The problem is that LOGPC would not evaluate its format string if
the logging message is not going to be printed, e.g. if a given
logging level is lower than the current one, or in case if
logging is not enabled at all.

The first two conditional calls to bitvec_read_field() are related
to CSN_PADDING_BITS, so that's not critical because padding is
always in the end of messages. The later two are related to
CSN_RECURSIVE_ARRAY and CSN_RECURSIVE_TARRAY respectively.

Let's use bitvec_get_uint() instead to keep readIndex unchanged.

Change-Id: Ia331048db9f790ca407fd341ced01df12d10a233
Fixes: OS#4375
---
M src/csn1.cpp
1 file changed, 4 insertions(+), 4 deletions(-)

Approvals:
  laforge: Looks good to me, approved
  pespin: Looks good to me, but someone else must approve
  Jenkins Builder: Verified



diff --git a/src/csn1.cpp b/src/csn1.cpp
index d358286..2f8bd03 100644
--- a/src/csn1.cpp
+++ b/src/csn1.cpp
@@ -1156,13 +1156,13 @@
             guint bits_to_handle = remaining_bits_len%8;
             if (bits_to_handle > 0)
             {
-	      LOGPC(DCSN1, LOGL_NOTICE, "%" PRIu64 "|", bitvec_read_field(vector, &readIndex, bits_to_handle));
+              LOGPC(DCSN1, LOGL_NOTICE, "%d|", bitvec_get_uint(vector, bits_to_handle));
               remaining_bits_len -= bits_to_handle;
               bit_offset += bits_to_handle;
             }
             else if (bits_to_handle == 0)
             {
-	      LOGPC(DCSN1, LOGL_NOTICE, "%" PRIu64 "|", bitvec_read_field(vector, &readIndex, 8));
+              LOGPC(DCSN1, LOGL_NOTICE, "%d|", bitvec_get_uint(vector, 8));
               remaining_bits_len -= 8;
               bit_offset += 8;
             }
@@ -1250,7 +1250,7 @@
           bit_offset += no_of_bits;
         }
 
-	LOGPC(DCSN1, LOGL_NOTICE, "%s = %u | ", pDescr->sz , (unsigned)bitvec_read_field(vector, &readIndex, 1));
+        LOGPC(DCSN1, LOGL_NOTICE, "%s = %d | ", pDescr->sz , bitvec_get_uint(vector, 1));
         /* existNextElement() returned FALSE, 1 bit consumed */
         bit_offset++;
         remaining_bits_len--;
@@ -1303,7 +1303,7 @@
           }
         }
 
-	LOGPC(DCSN1, LOGL_NOTICE, "%s = %u | ", pDescr->sz , (unsigned)bitvec_read_field(vector, &readIndex, 1));
+        LOGPC(DCSN1, LOGL_NOTICE, "%s = %u | ", pDescr->sz , bitvec_get_uint(vector, 1));
 
         /* existNextElement() returned FALSE, 1 bit consumed */
         bit_offset++;

-- 
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/17091
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: Ia331048db9f790ca407fd341ced01df12d10a233
Gerrit-Change-Number: 17091
Gerrit-PatchSet: 3
Gerrit-Owner: fixeria <axilirator at gmail.com>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <axilirator at gmail.com>
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20200206/24e7be10/attachment.htm>


More information about the gerrit-log mailing list