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

[fixeria]

fixeria has uploaded this change for review. ( 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 OS4375). 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(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/91/17091/1

diff --git a/src/csn1.cpp b/src/csn1.cpp
index 1a3a530..12f9819 100644
--- a/src/csn1.cpp
+++ b/src/csn1.cpp
@@ -1153,13 +1153,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;
             }
@@ -1247,7 +1247,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--;
@@ -1300,7 +1300,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: 1
Gerrit-Owner: fixeria <axilirator at gmail.com>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20200204/a793b29b/attachment.htm>