[S] Change in osmo-mgw[master]: client SDP: more verbose error logging
neels has submitted this change. (
https://gerrit.osmocom.org/c/osmo-mgw/+/35421?usp=email )
Change subject: client SDP: more verbose error logging
..
client SDP: more verbose error logging
So far it was pure guess work to find out why a message fails.
Change-Id: Ibc6343db82281789004c140ba98d99e5f6f73d83
---
M src/libosmo-mgcp-client/mgcp_client.c
1 file changed, 27 insertions(+), 5 deletions(-)
Approvals:
dexter: Looks good to me, but someone else must approve
Jenkins Builder: Verified
neels: Looks good to me, approved
laforge: Looks good to me, but someone else must approve
diff --git a/src/libosmo-mgcp-client/mgcp_client.c
b/src/libosmo-mgcp-client/mgcp_client.c
index 8df65cd..489ce69 100644
--- a/src/libosmo-mgcp-client/mgcp_client.c
+++ b/src/libosmo-mgcp-client/mgcp_client.c
@@ -329,20 +329,31 @@
errno = 0;
pt = strtoul(pt_str, _end, 0);
if ((errno == ERANGE && pt == ULONG_MAX) || (errno && !pt) ||
- pt_str == pt_end)
+ pt_str == pt_end) {
+ LOGP(DLMGCP, LOGL_ERROR, "SDP: cannot parse payload
type number from '%s'\n", pt_str);
goto response_parse_failure_pt;
+ }
- if (pt >> 7) /* PT is 7 bit field, higher values not allowed */
+ /* PT is 7 bit field, higher values not allowed */
+ if (pt >> 7) {
+ LOGP(DLMGCP, LOGL_ERROR, "SDP: payload type number out
of range: %lu > 127\n", pt);
goto response_parse_failure_pt;
+ }
/* Do not allow duplicate payload types */
- for (i = 0; i < ptmap_len; i++)
- if (r->ptmap[i].pt == pt)
+ for (i = 0; i < ptmap_len; i++) {
+ if (r->ptmap[i].pt == pt) {
+ LOGP(DLMGCP, LOGL_ERROR, "SDP: payload type
number %lu listed twice\n", pt);
goto response_parse_failure_pt;
+ }
+ }
/* Do not allow excessive payload types */
- if (ptmap_len >= ARRAY_SIZE(r->ptmap))
+ if (ptmap_len >= ARRAY_SIZE(r->ptmap)) {
+ LOGP(DLMGCP, LOGL_ERROR,
+"SDP: can parse only up to %zu payload type
numbers\n", ARRAY_SIZE(r->ptmap));
goto response_parse_failure_pt;
+ }
/* Some payload type numbers imply a specific codec. For those,
using the PT number as enum mgcp_codecs
* yields the correct result. If no more specific information
on the codec follows in "a=rtpmap:N"
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/35421?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: Ibc6343db82281789004c140ba98d99e5f6f73d83
Gerrit-Change-Number: 35421
Gerrit-PatchSet: 2
Gerrit-Owner: neels
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter
Gerrit-Reviewer: laforge
Gerrit-Reviewer: neels
Gerrit-CC: pespin
Gerrit-MessageType: merged
[S] Change in osmo-mgw[master]: client SDP: more verbose error logging
Attention is currently required from: pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/35421?usp=email ) Change subject: client SDP: more verbose error logging .. Patch Set 2: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/35421?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: Ibc6343db82281789004c140ba98d99e5f6f73d83 Gerrit-Change-Number: 35421 Gerrit-PatchSet: 2 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-CC: pespin Gerrit-Attention: pespin Gerrit-Comment-Date: Wed, 10 Jan 2024 08:00:32 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[S] Change in osmo-mgw[master]: client SDP: more verbose error logging
Attention is currently required from: neels, pespin. dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/35421?usp=email ) Change subject: client SDP: more verbose error logging .. Patch Set 2: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/35421?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: Ibc6343db82281789004c140ba98d99e5f6f73d83 Gerrit-Change-Number: 35421 Gerrit-PatchSet: 2 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter Gerrit-Reviewer: laforge Gerrit-CC: pespin Gerrit-Attention: neels Gerrit-Attention: pespin Gerrit-Comment-Date: Fri, 05 Jan 2024 11:48:48 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[S] Change in osmo-mgw[master]: client SDP: more verbose error logging
Attention is currently required from: pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/35421?usp=email ) Change subject: client SDP: more verbose error logging .. Patch Set 1: (2 comments) File src/libosmo-mgcp-client/mgcp_client.c: https://gerrit.osmocom.org/c/osmo-mgw/+/35421/comment/4b9f502b_43a4d7bf PS1, Line 376: LOGP(DLMGCP, LOGL_ERROR, > it would be the other one below, with the "_pt:" label... Done https://gerrit.osmocom.org/c/osmo-mgw/+/35421/comment/1fe4df57_b62da484 PS1, Line 382: "Failed to parse SDP parameter payload types (%s)\n", line); > ...this one. […] to recap, I don't agree with dropping log of the SDP string that failed. i'd just leave it as it is, but we could reduce to INFO or DEBUG now? let me know or i'll resolve this soon. -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/35421?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: Ibc6343db82281789004c140ba98d99e5f6f73d83 Gerrit-Change-Number: 35421 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-CC: pespin Gerrit-Attention: pespin Gerrit-Comment-Date: Thu, 04 Jan 2024 23:55:23 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels Comment-In-Reply-To: pespin Gerrit-MessageType: comment
[S] Change in osmo-mgw[master]: client SDP: more verbose error logging
Attention is currently required from: neels, pespin. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/35421?usp=email ) Change subject: client SDP: more verbose error logging .. Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/35421?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: Ibc6343db82281789004c140ba98d99e5f6f73d83 Gerrit-Change-Number: 35421 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-CC: pespin Gerrit-Attention: neels Gerrit-Attention: pespin Gerrit-Comment-Date: Sat, 23 Dec 2023 06:57:28 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[S] Change in osmo-mgw[master]: client SDP: more verbose error logging
Attention is currently required from: pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/35421?usp=email ) Change subject: client SDP: more verbose error logging .. Patch Set 1: (2 comments) File src/libosmo-mgcp-client/mgcp_client.c: https://gerrit.osmocom.org/c/osmo-mgw/+/35421/comment/5eb76095_3dbdd07e PS1, Line 376: LOGP(DLMGCP, LOGL_ERROR, > So now this log line can be dropped? it makes no sense to print twice imho. it would be the other one below, with the "_pt:" label... https://gerrit.osmocom.org/c/osmo-mgw/+/35421/comment/9dcb8c57_78d220c1 PS1, Line 382: "Failed to parse SDP parameter payload types (%s)\n", line); ...this one. I think it's not harmful to also print the SDP string that failed to parse. If you think it's too verbose for LOGL_ERROR, then we can put either the above or this one on DEBUG log...? -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/35421?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: Ibc6343db82281789004c140ba98d99e5f6f73d83 Gerrit-Change-Number: 35421 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-CC: pespin Gerrit-Attention: pespin Gerrit-Comment-Date: Sat, 23 Dec 2023 06:13:02 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin Gerrit-MessageType: comment
[S] Change in osmo-mgw[master]: client SDP: more verbose error logging
Attention is currently required from: neels. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/35421?usp=email ) Change subject: client SDP: more verbose error logging .. Patch Set 1: (1 comment) File src/libosmo-mgcp-client/mgcp_client.c: https://gerrit.osmocom.org/c/osmo-mgw/+/35421/comment/0ec36c09_ca30d24f PS1, Line 376: LOGP(DLMGCP, LOGL_ERROR, So now this log line can be dropped? it makes no sense to print twice imho. -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/35421?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: Ibc6343db82281789004c140ba98d99e5f6f73d83 Gerrit-Change-Number: 35421 Gerrit-PatchSet: 1 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-CC: pespin Gerrit-Attention: neels Gerrit-Comment-Date: Fri, 22 Dec 2023 10:00:33 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
[S] Change in osmo-mgw[master]: client SDP: more verbose error logging
neels has uploaded this change for review. (
https://gerrit.osmocom.org/c/osmo-mgw/+/35421?usp=email )
Change subject: client SDP: more verbose error logging
..
client SDP: more verbose error logging
So far it was pure guess work to find out why a message fails.
Change-Id: Ibc6343db82281789004c140ba98d99e5f6f73d83
---
M src/libosmo-mgcp-client/mgcp_client.c
1 file changed, 27 insertions(+), 5 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/21/35421/1
diff --git a/src/libosmo-mgcp-client/mgcp_client.c
b/src/libosmo-mgcp-client/mgcp_client.c
index 8df65cd..489ce69 100644
--- a/src/libosmo-mgcp-client/mgcp_client.c
+++ b/src/libosmo-mgcp-client/mgcp_client.c
@@ -329,20 +329,31 @@
errno = 0;
pt = strtoul(pt_str, _end, 0);
if ((errno == ERANGE && pt == ULONG_MAX) || (errno && !pt) ||
- pt_str == pt_end)
+ pt_str == pt_end) {
+ LOGP(DLMGCP, LOGL_ERROR, "SDP: cannot parse payload
type number from '%s'\n", pt_str);
goto response_parse_failure_pt;
+ }
- if (pt >> 7) /* PT is 7 bit field, higher values not allowed */
+ /* PT is 7 bit field, higher values not allowed */
+ if (pt >> 7) {
+ LOGP(DLMGCP, LOGL_ERROR, "SDP: payload type number out
of range: %lu > 127\n", pt);
goto response_parse_failure_pt;
+ }
/* Do not allow duplicate payload types */
- for (i = 0; i < ptmap_len; i++)
- if (r->ptmap[i].pt == pt)
+ for (i = 0; i < ptmap_len; i++) {
+ if (r->ptmap[i].pt == pt) {
+ LOGP(DLMGCP, LOGL_ERROR, "SDP: payload type
number %lu listed twice\n", pt);
goto response_parse_failure_pt;
+ }
+ }
/* Do not allow excessive payload types */
- if (ptmap_len >= ARRAY_SIZE(r->ptmap))
+ if (ptmap_len >= ARRAY_SIZE(r->ptmap)) {
+ LOGP(DLMGCP, LOGL_ERROR,
+"SDP: can parse only up to %zu payload type
numbers\n", ARRAY_SIZE(r->ptmap));
goto response_parse_failure_pt;
+ }
/* Some payload type numbers imply a specific codec. For those,
using the PT number as enum mgcp_codecs
* yields the correct result. If no more specific information
on the codec follows in "a=rtpmap:N"
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/35421?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: Ibc6343db82281789004c140ba98d99e5f6f73d83
Gerrit-Change-Number: 35421
Gerrit-PatchSet: 1
Gerrit-Owner: neels
Gerrit-MessageType: newchange
