<p>pespin has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.osmocom.org/c/osmo-pcu/+/17088">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">Fix trailing newline mess with LOGP(C) in rlcmac/csn1<br><br>Output was incorrect before this patch. LOPC was being called without<br>having any initial LOGP, and trailing newline was usually missing at the<br>end.<br><br>Since csnDecoder/encoder functions are recursive, it's difficult to<br>handle logging state in a coherent way inside them. Let's better simply<br>control start/end of logging related topics in the callers of those<br>functions, and simply use LOGPC everywhere in csn1.cpp.<br><br>Change-Id: I50da7560939fac360b7545e2a6bfaf45ed0c4832<br>---<br>M src/csn1.cpp<br>M src/gsm_rlcmac.cpp<br>2 files changed, 38 insertions(+), 1 deletion(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/88/17088/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/csn1.cpp b/src/csn1.cpp</span><br><span>index e97df92..d59030c 100644</span><br><span>--- a/src/csn1.cpp</span><br><span>+++ b/src/csn1.cpp</span><br><span>@@ -114,8 +114,9 @@</span><br><span> static gint16 ProcessError_impl(const char *file, int line, unsigned readIndex,</span><br><span> const char* sz, gint16 err, const CSN_DESCR* pDescr)</span><br><span> {</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Don't add trailing newline, top caller is responsible for appending it */</span><br><span> if (err != CSN_OK)</span><br><span style="color: hsl(0, 100%, 40%);">- LOGPSRC(DCSN1, LOGL_ERROR, file, line, "%s: error %s (%d) at %s (idx %d)\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPSRC(DCSN1, LOGL_ERROR, file, line, "%s: error %s (%d) at %s (idx %d)",</span><br><span> sz, get_value_string(csn1_error_names, err), err,</span><br><span> pDescr ? pDescr->sz : "-", readIndex);</span><br><span> return err;</span><br><span>diff --git a/src/gsm_rlcmac.cpp b/src/gsm_rlcmac.cpp</span><br><span>index 25f9072..753960b 100644</span><br><span>--- a/src/gsm_rlcmac.cpp</span><br><span>+++ b/src/gsm_rlcmac.cpp</span><br><span>@@ -4825,11 +4825,15 @@</span><br><span> LOGP(DRLCMACDATA, LOGL_NOTICE, "Payload Type: RESERVED (3)");</span><br><span> return -1;</span><br><span> }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> data->NrOfBits = 23 * 8;</span><br><span> csnStreamInit(&ar, 0, data->NrOfBits);</span><br><span> readIndex += 6;</span><br><span> data->u.MESSAGE_TYPE = bitvec_read_field(vector, &readIndex, 6);</span><br><span> readIndex = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* recursive csnStreamDecoder call uses LOGPC everywhere, so we need to start the log somewhere... */</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DCSN1, LOGL_INFO, "csnStreamDecoder (type=%d): ", data->u.MESSAGE_TYPE);</span><br><span> switch (data->u.MESSAGE_TYPE)</span><br><span> {</span><br><span> case MT_PACKET_CELL_CHANGE_FAILURE:</span><br><span>@@ -4913,6 +4917,10 @@</span><br><span> break;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ /* recursive csnStreamDecoder call uses LOGPC everywhere without trailing</span><br><span style="color: hsl(120, 100%, 40%);">+ newline, so as a caller we are responisble for submitting it */</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPC(DCSN1, LOGL_INFO, "\n");</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> if (ret > 0) {</span><br><span> LOGP(DRLCMACDATA, LOGL_NOTICE, "Got %d remaining bits unhandled by decoder at the end of bitvec\n", ret);</span><br><span> ret = 0;</span><br><span>@@ -4983,6 +4991,9 @@</span><br><span> </span><br><span> csnStreamInit(&ar, bit_offset, bit_length);</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ /* recursive csnStreamDecoder call uses LOGPC everywhere, so we need to start the log somewhere... */</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DCSN1, LOGL_INFO, "csnStreamDecoder (type=%d): ", data->u.MESSAGE_TYPE);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> switch (data->u.MESSAGE_TYPE)</span><br><span> {</span><br><span> case MT_PACKET_ACCESS_REJECT:</span><br><span>@@ -5115,6 +5126,10 @@</span><br><span> break;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ /* recursive csnStreamDecoder call uses LOGPC everywhere without trailing</span><br><span style="color: hsl(120, 100%, 40%);">+ newline, so as a caller we are responisble for submitting it */</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPC(DCSN1, LOGL_INFO, "\n");</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> if (ret > 0) {</span><br><span> LOGP(DRLCMACDATA, LOGL_NOTICE, "Got %d remaining bits unhandled by decoder at the end of bitvec\n", ret);</span><br><span> ret = 0;</span><br><span>@@ -5133,6 +5148,9 @@</span><br><span> data->NrOfBits = 23 * 8;</span><br><span> csnStreamInit(&ar, 0, data->NrOfBits);</span><br><span> writeIndex = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* recursive csnStreamEncoder call uses LOGPC everywhere, so we need to start the log somewhere... */</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DCSN1, LOGL_INFO, "csnStreamEncoder (type=%d): ", data->u.MESSAGE_TYPE);</span><br><span> switch (data->u.MESSAGE_TYPE)</span><br><span> {</span><br><span> case MT_PACKET_CELL_CHANGE_FAILURE:</span><br><span>@@ -5216,6 +5234,10 @@</span><br><span> break;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ /* recursive csnStreamDecoder call uses LOGPC everywhere without trailing</span><br><span style="color: hsl(120, 100%, 40%);">+ newline, so as a caller we are responisble for submitting it */</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPC(DCSN1, LOGL_INFO, "\n");</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> if (ret > 0) {</span><br><span> LOGP(DRLCMACDATA, LOGL_NOTICE, "Got %d remaining bits unhandled by encoder at the end of bitvec\n", ret);</span><br><span> ret = 0;</span><br><span>@@ -5283,6 +5305,9 @@</span><br><span> </span><br><span> csnStreamInit(&ar, bit_offset, bit_length);</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* recursive csnStreamEncoder call uses LOGPC everywhere, so we need to start the log somewhere... */</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DCSN1, LOGL_INFO, "csnStreamEncoder (type=%d): ", data->u.MESSAGE_TYPE);</span><br><span> switch (data->u.MESSAGE_TYPE)</span><br><span> {</span><br><span> case MT_PACKET_ACCESS_REJECT:</span><br><span>@@ -5415,6 +5440,10 @@</span><br><span> break;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ /* recursive csnStreamDecoder call uses LOGPC everywhere without trailing</span><br><span style="color: hsl(120, 100%, 40%);">+ newline, so as a caller we are responisble for submitting it */</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPC(DCSN1, LOGL_INFO, "\n");</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> if (ret > 0) {</span><br><span> LOGP(DRLCMACDATA, LOGL_NOTICE, "Got %d remaining bits unhandled by encoder at the end of bitvec\n", ret);</span><br><span> ret = 0;</span><br><span>@@ -5566,8 +5595,15 @@</span><br><span> unsigned readIndex = 0;</span><br><span> </span><br><span> csnStreamInit(&ar, 0, 8 * vector->data_len);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* recursive csnStreamEncoder call uses LOGPC everywhere, so we need to start the log somewhere... */</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DCSN1, LOGL_INFO, "csnStreamEncoder (RAcap): ");</span><br><span> ret = csnStreamDecoder(&ar, CSNDESCR(MS_Radio_Access_capability_t), vector, readIndex, data);</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ /* recursive csnStreamDecoder call uses LOGPC everywhere without trailing</span><br><span style="color: hsl(120, 100%, 40%);">+ newline, so as a caller we are responisble for submitting it */</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPC(DCSN1, LOGL_INFO, "\n");</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> if (ret > 0) {</span><br><span> LOGP(DRLCMACDATA, LOGL_NOTICE, "Got %d remaining bits unhandled by decoder at the end of bitvec\n", ret);</span><br><span> ret = 0;</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-pcu/+/17088">change 17088</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/c/osmo-pcu/+/17088"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: osmo-pcu </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I50da7560939fac360b7545e2a6bfaf45ed0c4832 </div>
<div style="display:none"> Gerrit-Change-Number: 17088 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>