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/+/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>