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.orglaforge 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>