Change in osmo-pcu[master]: Fix trailing newline mess with LOGP(C) in rlcmac/csn1

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
Sat Feb 8 11:03:13 UTC 2020 

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

Change subject: Fix trailing newline mess with LOGP(C) in rlcmac/csn1
......................................................................

Fix trailing newline mess with LOGP(C) in rlcmac/csn1

Output was incorrect before this patch. LOPC was being called without
having any initial LOGP, and trailing newline was usually missing at the
end.

Since csnDecoder/encoder functions are recursive, it's difficult to
handle logging state in a coherent way inside them. Let's better simply
control start/end of logging related topics in the callers of those
functions, and simply use LOGPC everywhere in csn1.cpp.

Change-Id: I50da7560939fac360b7545e2a6bfaf45ed0c4832
---
M src/csn1.cpp
M src/gsm_rlcmac.cpp
2 files changed, 38 insertions(+), 1 deletion(-)

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



diff --git a/src/csn1.cpp b/src/csn1.cpp
index 2f8bd03..7e75972 100644
--- a/src/csn1.cpp
+++ b/src/csn1.cpp
@@ -114,8 +114,9 @@
 static gint16 ProcessError_impl(const char *file, int line, unsigned readIndex,
                                 const char* sz, gint16 err, const CSN_DESCR* pDescr)
 {
+  /* Don't add trailing newline, top caller is responsible for appending it */
   if (err != CSN_OK)
-    LOGPSRC(DCSN1, LOGL_ERROR, file, line, "%s: error %s (%d) at %s (idx %d)\n",
+    LOGPSRC(DCSN1, LOGL_ERROR, file, line, "%s: error %s (%d) at %s (idx %d)",
             sz, get_value_string(csn1_error_names, err), err,
             pDescr ? pDescr->sz : "-", readIndex);
   return err;
diff --git a/src/gsm_rlcmac.cpp b/src/gsm_rlcmac.cpp
index e594092..8d846a1 100644
--- a/src/gsm_rlcmac.cpp
+++ b/src/gsm_rlcmac.cpp
@@ -4825,11 +4825,15 @@
     LOGP(DRLCMACDATA, LOGL_NOTICE, "Payload Type: RESERVED (3)\n");
     return CSN_ERROR_GENERAL;
   }
+
   data->NrOfBits = 23 * 8;
   csnStreamInit(&ar, 0, data->NrOfBits);
   readIndex += 6;
   data->u.MESSAGE_TYPE = bitvec_read_field(vector, &readIndex, 6);
   readIndex = 0;
+
+  /* recursive csnStreamDecoder call uses LOGPC everywhere, so we need to start the log somewhere... */
+  LOGP(DCSN1, LOGL_INFO, "csnStreamDecoder (type=%d): ", data->u.MESSAGE_TYPE);
   switch (data->u.MESSAGE_TYPE)
   {
     case MT_PACKET_CELL_CHANGE_FAILURE:
@@ -4913,6 +4917,10 @@
       break;
   }
 
+  /* recursive csnStreamDecoder call uses LOGPC everywhere without trailing
+     newline, so as a caller we are responisble for submitting it */
+  LOGPC(DCSN1, LOGL_INFO, "\n");
+
   if (ret > 0) {
     LOGP(DRLCMACDATA, LOGL_NOTICE, "Got %d remaining bits unhandled by decoder at the end of bitvec\n", ret);
     ret = 0;
@@ -4983,6 +4991,9 @@
 
   csnStreamInit(&ar, bit_offset, bit_length);
 
+  /* recursive csnStreamDecoder call uses LOGPC everywhere, so we need to start the log somewhere... */
+  LOGP(DCSN1, LOGL_INFO, "csnStreamDecoder (type=%d): ", data->u.MESSAGE_TYPE);
+
   switch (data->u.MESSAGE_TYPE)
   {
     case MT_PACKET_ACCESS_REJECT:
@@ -5115,6 +5126,10 @@
       break;
   }
 
+  /* recursive csnStreamDecoder call uses LOGPC everywhere without trailing
+     newline, so as a caller we are responisble for submitting it */
+  LOGPC(DCSN1, LOGL_INFO, "\n");
+
   if (ret > 0) {
     LOGP(DRLCMACDATA, LOGL_NOTICE, "Got %d remaining bits unhandled by decoder at the end of bitvec\n", ret);
     ret = 0;
@@ -5133,6 +5148,9 @@
   data->NrOfBits = 23 * 8;
   csnStreamInit(&ar, 0, data->NrOfBits);
   writeIndex = 0;
+
+  /* recursive csnStreamEncoder call uses LOGPC everywhere, so we need to start the log somewhere... */
+  LOGP(DCSN1, LOGL_INFO, "csnStreamEncoder (type=%d): ", data->u.MESSAGE_TYPE);
   switch (data->u.MESSAGE_TYPE)
   {
     case MT_PACKET_CELL_CHANGE_FAILURE:
@@ -5216,6 +5234,10 @@
       break;
   }
 
+  /* recursive csnStreamDecoder call uses LOGPC everywhere without trailing
+     newline, so as a caller we are responisble for submitting it */
+  LOGPC(DCSN1, LOGL_INFO, "\n");
+
   if (ret > 0) {
     LOGP(DRLCMACDATA, LOGL_NOTICE, "Got %d remaining bits unhandled by encoder at the end of bitvec\n", ret);
     ret = 0;
@@ -5283,6 +5305,9 @@
 
   csnStreamInit(&ar, bit_offset, bit_length);
 
+
+  /* recursive csnStreamEncoder call uses LOGPC everywhere, so we need to start the log somewhere... */
+  LOGP(DCSN1, LOGL_INFO, "csnStreamEncoder (type=%d): ", data->u.MESSAGE_TYPE);
   switch (data->u.MESSAGE_TYPE)
   {
     case MT_PACKET_ACCESS_REJECT:
@@ -5415,6 +5440,10 @@
       break;
   }
 
+  /* recursive csnStreamDecoder call uses LOGPC everywhere without trailing
+     newline, so as a caller we are responisble for submitting it */
+  LOGPC(DCSN1, LOGL_INFO, "\n");
+
   if (ret > 0) {
     LOGP(DRLCMACDATA, LOGL_NOTICE, "Got %d remaining bits unhandled by encoder at the end of bitvec\n", ret);
     ret = 0;
@@ -5566,8 +5595,15 @@
   unsigned readIndex = 0;
 
   csnStreamInit(&ar, 0, 8 * vector->data_len);
+
+  /* recursive csnStreamEncoder call uses LOGPC everywhere, so we need to start the log somewhere... */
+  LOGP(DCSN1, LOGL_INFO, "csnStreamEncoder (RAcap): ");
   ret = csnStreamDecoder(&ar, CSNDESCR(MS_Radio_Access_capability_t), vector, readIndex, data);
 
+  /* recursive csnStreamDecoder call uses LOGPC everywhere without trailing
+     newline, so as a caller we are responisble for submitting it */
+  LOGPC(DCSN1, LOGL_INFO, "\n");
+
   if (ret > 0) {
     LOGP(DRLCMACDATA, LOGL_NOTICE, "Got %d remaining bits unhandled by decoder at the end of bitvec\n", ret);
     ret = 0;

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

Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I50da7560939fac360b7545e2a6bfaf45ed0c4832
Gerrit-Change-Number: 17088
Gerrit-PatchSet: 5
Gerrit-Owner: pespin <pespin at sysmocom.de>
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/20200208/1fa82470/attachment.htm>

More information about the gerrit-log mailing list