Change in osmo-sip-connector[master]: logging from sofia: add missing newline
laforge has submitted this change. (
https://gerrit.osmocom.org/c/osmo-sip-connector/+/16220 )
Change subject: logging from sofia: add missing newline
..
logging from sofia: add missing newline
Sometimes, logging from sofia lacks the final newline character, messing up log
output. First snprintf() to a buffer, add '\n' if necessary and then log.
Change-Id: Ia26c0b57a0166cf7de87c49471ce6f528a366dd5
---
M src/sip.c
1 file changed, 13 insertions(+), 1 deletion(-)
Approvals:
laforge: Looks good to me, approved
pespin: Looks good to me, but someone else must approve
Jenkins Builder: Verified
diff --git a/src/sip.c b/src/sip.c
index 2b28b4e..c635542 100644
--- a/src/sip.c
+++ b/src/sip.c
@@ -666,7 +666,19 @@
* the log handler call-back function, so we have no clue what log
level the
* currently logged message was sent for :( As a result, we can only
use one
* hard-coded LOGL_NOTICE here */
- osmo_vlogp(DSIP, LOGL_NOTICE, "", 0, 0, fmt, ap);
+ if (!log_check_level(DSIP, LOGL_NOTICE))
+ return;
+ /* The sofia-sip log line *sometimes* lacks a terminating '\n'. Add it.
*/
+ char log_line[256];
+ int rc = vsnprintf(log_line, sizeof(log_line), fmt, ap);
+ if (rc > 0) {
+ /* since we're explicitly checking for sizeof(log_line), we can
use vsnprintf()'s return value (which,
+* alone, would possibly cause writing past the buffer's end).
*/
+ char *end = log_line + OSMO_MIN(rc, sizeof(log_line) - 2);
+ osmo_strlcpy(end, "\n", 2);
+ LOGP(DSIP, LOGL_NOTICE, "%s", log_line);
+ } else
+ LOGP(DSIP, LOGL_NOTICE, "unknown logging from sip\n");
}
void sip_agent_init(struct sip_agent *agent, struct app_config *app)
--
To view, visit https://gerrit.osmocom.org/c/osmo-sip-connector/+/16220
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-sip-connector
Gerrit-Branch: master
Gerrit-Change-Id: Ia26c0b57a0166cf7de87c49471ce6f528a366dd5
Gerrit-Change-Number: 16220
Gerrit-PatchSet: 3
Gerrit-Owner: neels
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge
Gerrit-Reviewer: neels
Gerrit-Reviewer: pespin
Gerrit-MessageType: merged
Change in osmo-sip-connector[master]: logging from sofia: add missing newline
laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-sip-connector/+/16220 ) Change subject: logging from sofia: add missing newline .. Patch Set 2: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-sip-connector/+/16220 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-sip-connector Gerrit-Branch: master Gerrit-Change-Id: Ia26c0b57a0166cf7de87c49471ce6f528a366dd5 Gerrit-Change-Number: 16220 Gerrit-PatchSet: 2 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-Comment-Date: Tue, 17 Dec 2019 13:52:20 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Change in osmo-sip-connector[master]: logging from sofia: add missing newline
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-sip-connector/+/16220 ) Change subject: logging from sofia: add missing newline .. Patch Set 2: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-sip-connector/+/16220 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-sip-connector Gerrit-Branch: master Gerrit-Change-Id: Ia26c0b57a0166cf7de87c49471ce6f528a366dd5 Gerrit-Change-Number: 16220 Gerrit-PatchSet: 2 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-Comment-Date: Thu, 28 Nov 2019 21:53:30 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Change in osmo-sip-connector[master]: logging from sofia: add missing newline
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-sip-connector/+/16220 ) Change subject: logging from sofia: add missing newline .. Patch Set 2: (1 comment) https://gerrit.osmocom.org/c/osmo-sip-connector/+/16220/1/src/sip.c File src/sip.c: https://gerrit.osmocom.org/c/osmo-sip-connector/+/16220/1/src/sip.c@674 PS1, Line 674: char *end = log_line + OSMO_MIN(strlen(log_line), sizeof(log_line) - 2); > Because of the OSMO_MIN() with the sizeof, you are actually correct. […] sure, you need to of course trim it to the size f the buffer if it's bigger, but still that's one comparison instead of counting 256 times :) -- To view, visit https://gerrit.osmocom.org/c/osmo-sip-connector/+/16220 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-sip-connector Gerrit-Branch: master Gerrit-Change-Id: Ia26c0b57a0166cf7de87c49471ce6f528a366dd5 Gerrit-Change-Number: 16220 Gerrit-PatchSet: 2 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-CC: pespin Gerrit-Comment-Date: Thu, 28 Nov 2019 21:51:28 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin Comment-In-Reply-To: neels Gerrit-MessageType: comment
Change in osmo-sip-connector[master]: logging from sofia: add missing newline
Hello laforge, Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-sip-connector/+/16220 to look at the new patch set (#2). Change subject: logging from sofia: add missing newline .. logging from sofia: add missing newline Sometimes, logging from sofia lacks the final newline character, messing up log output. First snprintf() to a buffer, add '\n' if necessary and then log. Change-Id: Ia26c0b57a0166cf7de87c49471ce6f528a366dd5 --- M src/sip.c 1 file changed, 13 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/osmo-sip-connector refs/changes/20/16220/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-sip-connector/+/16220 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-sip-connector Gerrit-Branch: master Gerrit-Change-Id: Ia26c0b57a0166cf7de87c49471ce6f528a366dd5 Gerrit-Change-Number: 16220 Gerrit-PatchSet: 2 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-CC: pespin Gerrit-MessageType: newpatchset
Change in osmo-sip-connector[master]: logging from sofia: add missing newline
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-sip-connector/+/16220 ) Change subject: logging from sofia: add missing newline .. Patch Set 1: (1 comment) https://gerrit.osmocom.org/c/osmo-sip-connector/+/16220/1/src/sip.c File src/sip.c: https://gerrit.osmocom.org/c/osmo-sip-connector/+/16220/1/src/sip.c@674 PS1, Line 674: char *end = log_line + OSMO_MIN(strlen(log_line), sizeof(log_line) - 2); > No need to strlen(log_line) here, you should have available information from > return of vsnprintf abo […] Because of the OSMO_MIN() with the sizeof, you are actually correct. Just to make sure for future patches: vsnprintf() returns the strlen that *would* be written if the buffer were large enough. If that OSMO_MIN() weren't there, vsnprintf's rc would *not* qualify to ensure staying inside the buf. -- To view, visit https://gerrit.osmocom.org/c/osmo-sip-connector/+/16220 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-sip-connector Gerrit-Branch: master Gerrit-Change-Id: Ia26c0b57a0166cf7de87c49471ce6f528a366dd5 Gerrit-Change-Number: 16220 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-CC: pespin Gerrit-Comment-Date: Thu, 28 Nov 2019 20:17:28 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin Gerrit-MessageType: comment
Change in osmo-sip-connector[master]: logging from sofia: add missing newline
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-sip-connector/+/16220 ) Change subject: logging from sofia: add missing newline .. Patch Set 1: (2 comments) https://gerrit.osmocom.org/c/osmo-sip-connector/+/16220/1//COMMIT_MSG Commit Message: https://gerrit.osmocom.org/c/osmo-sip-connector/+/16220/1//COMMIT_MSG@9 PS1, Line 9: Sometimes, lgging from sofia lacks the final newline character, messing up log logging https://gerrit.osmocom.org/c/osmo-sip-connector/+/16220/1/src/sip.c File src/sip.c: https://gerrit.osmocom.org/c/osmo-sip-connector/+/16220/1/src/sip.c@674 PS1, Line 674: char *end = log_line + OSMO_MIN(strlen(log_line), sizeof(log_line) - 2); No need to strlen(log_line) here, you should have available information from return of vsnprintf above. -- To view, visit https://gerrit.osmocom.org/c/osmo-sip-connector/+/16220 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-sip-connector Gerrit-Branch: master Gerrit-Change-Id: Ia26c0b57a0166cf7de87c49471ce6f528a366dd5 Gerrit-Change-Number: 16220 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-CC: pespin Gerrit-Comment-Date: Wed, 27 Nov 2019 09:21:52 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
Change in osmo-sip-connector[master]: logging from sofia: add missing newline
laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-sip-connector/+/16220 ) Change subject: logging from sofia: add missing newline .. Patch Set 1: Code-Review+1 We could probably do a vlogp followed by a LOGPC to avoid yet another sprintf, but up to you -- To view, visit https://gerrit.osmocom.org/c/osmo-sip-connector/+/16220 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-sip-connector Gerrit-Branch: master Gerrit-Change-Id: Ia26c0b57a0166cf7de87c49471ce6f528a366dd5 Gerrit-Change-Number: 16220 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Comment-Date: Wed, 27 Nov 2019 09:06:10 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Change in osmo-sip-connector[master]: logging from sofia: add missing newline
neels has uploaded this change for review. (
https://gerrit.osmocom.org/c/osmo-sip-connector/+/16220 )
Change subject: logging from sofia: add missing newline
..
logging from sofia: add missing newline
Sometimes, lgging from sofia lacks the final newline character, messing up log
output. First snprintf() to a buffer, add '\n' if necessary and then log.
Change-Id: Ia26c0b57a0166cf7de87c49471ce6f528a366dd5
---
M src/sip.c
1 file changed, 10 insertions(+), 1 deletion(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-sip-connector
refs/changes/20/16220/1
diff --git a/src/sip.c b/src/sip.c
index 2b28b4e..5eaa94b 100644
--- a/src/sip.c
+++ b/src/sip.c
@@ -666,7 +666,16 @@
* the log handler call-back function, so we have no clue what log
level the
* currently logged message was sent for :( As a result, we can only
use one
* hard-coded LOGL_NOTICE here */
- osmo_vlogp(DSIP, LOGL_NOTICE, "", 0, 0, fmt, ap);
+ if (!log_check_level(DSIP, LOGL_NOTICE))
+ return;
+ /* The sofia-sip log line *sometimes* lacks a terminating '\n'. Add it.
*/
+ char log_line[256];
+ if (vsnprintf(log_line, sizeof(log_line), fmt, ap) > 0) {
+ char *end = log_line + OSMO_MIN(strlen(log_line),
sizeof(log_line) - 2);
+ osmo_strlcpy(end, "\n", 2);
+ LOGP(DSIP, LOGL_NOTICE, "%s", log_line);
+ } else
+ LOGP(DSIP, LOGL_NOTICE, "unknown logging from sip\n");
}
void sip_agent_init(struct sip_agent *agent, struct app_config *app)
--
To view, visit https://gerrit.osmocom.org/c/osmo-sip-connector/+/16220
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-sip-connector
Gerrit-Branch: master
Gerrit-Change-Id: Ia26c0b57a0166cf7de87c49471ce6f528a366dd5
Gerrit-Change-Number: 16220
Gerrit-PatchSet: 1
Gerrit-Owner: neels
Gerrit-MessageType: newchange
