neels has submitted this change and it was merged. (
https://gerrit.osmocom.org/c/osmo-mgw/+/15138 )
Change subject: mgcp_codec: codec_set(): log about all possible errors
......................................................................
mgcp_codec: codec_set(): log about all possible errors
In codec_set(), for each 'goto error', log the specific error cause.
Also add a TODO and a FIXME comment about inventing dynamic payload type
numbers.
Change-Id: I0b44b574c814882b6f8ae7cd738a6f481cd721fd
---
M src/libosmo-mgcp/mgcp_codec.c
1 file changed, 25 insertions(+), 9 deletions(-)
Approvals:
Jenkins Builder: Verified
osmith: Looks good to me, but someone else must approve
pespin: Looks good to me, approved
diff --git a/src/libosmo-mgcp/mgcp_codec.c b/src/libosmo-mgcp/mgcp_codec.c
index 7c8f6d5..704b7e6 100644
--- a/src/libosmo-mgcp/mgcp_codec.c
+++ b/src/libosmo-mgcp/mgcp_codec.c
@@ -138,12 +138,13 @@
if (payload_type != PTYPE_UNDEFINED) {
/* Make sure we do not get any reserved or undefined type
numbers */
/* See also:
https://www.iana.org/assignments/rtp-parameters/rtp-parameters.xhtml */
- if (payload_type == 1 || payload_type == 2 || payload_type ==
19)
+ if ((payload_type == 1 || payload_type == 2 || payload_type ==
19)
+ || (payload_type >= 72 && payload_type <= 76)
+ || (payload_type >= 127)) {
+ LOGP(DLMGCP, LOGL_ERROR, "Cannot add codec, payload
type number %d is reserved\n",
+ payload_type);
goto error;
- if (payload_type >= 72 && payload_type <= 76)
- goto error;
- if (payload_type >= 127)
- goto error;
+ }
codec->payload_type = payload_type;
}
@@ -169,6 +170,8 @@
/* The given payload type is not known to us, or it
* it is a dynamic payload type for which we do not
* know the audio name. We must give up here */
+ LOGP(DLMGCP, LOGL_ERROR, "No audio codec name given,
and payload type %d unknown\n",
+ payload_type);
goto error;
}
}
@@ -176,16 +179,23 @@
/* Now we extract the codec subtype name, rate and channels. The latter
* two are optional. If they are not present we use the safe defaults
* above. */
- if (strlen(audio_name) > sizeof(audio_codec))
+ if (strlen(audio_name) > sizeof(audio_codec)) {
+ LOGP(DLMGCP, LOGL_ERROR, "Audio codec too long: %s\n",
osmo_quote_str(audio_name, -1));
goto error;
+ }
channels = DEFAULT_RTP_AUDIO_DEFAULT_CHANNELS;
rate = DEFAULT_RTP_AUDIO_DEFAULT_RATE;
- if (sscanf(audio_name, "%63[^/]/%d/%d", audio_codec, &rate, &channels)
< 1)
+ if (sscanf(audio_name, "%63[^/]/%d/%d", audio_codec, &rate, &channels)
< 1) {
+ LOGP(DLMGCP, LOGL_ERROR, "Invalid audio codec: %s\n",
osmo_quote_str(audio_name, -1));
goto error;
+ }
/* Note: We only accept configurations with one audio channel! */
- if (channels != 1)
+ if (channels != 1) {
+ LOGP(DLMGCP, LOGL_ERROR, "Cannot handle audio codec with more
than one channel: %s\n",
+ osmo_quote_str(audio_name, -1));
goto error;
+ }
codec->rate = rate;
codec->channels = channels;
@@ -203,6 +213,7 @@
/* Derive the payload type if it is unknown */
if (codec->payload_type == PTYPE_UNDEFINED) {
+ /* TODO: This is semi dead code, see OS#4150 */
/* For the known codecs from the static range we restore
* the IANA or 3GPP assigned payload type number */
@@ -238,9 +249,14 @@
* 110 onwards 3gpp defines prefered codec types, which are
* also fixed, see above) */
if (codec->payload_type < 0) {
+ /* FIXME: pt_offset is completely unrelated and useless
here, any of those numbers may already
+ * have been added to the codecs. Instead, there should
be an iterator checking for an actually
+ * unused dynamic payload type number. */
codec->payload_type = 96 + pt_offset;
- if (codec->payload_type > 109)
+ if (codec->payload_type > 109) {
+ LOGP(DLMGCP, LOGL_ERROR, "Ran out of payload
type numbers to assign dynamically\n");
goto error;
+ }
}
}
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/15138
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I0b44b574c814882b6f8ae7cd738a6f481cd721fd
Gerrit-Change-Number: 15138
Gerrit-PatchSet: 5
Gerrit-Owner: neels <nhofm...@sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels <nhofm...@sysmocom.de>
Gerrit-Reviewer: osmith <osm...@sysmocom.de>
Gerrit-Reviewer: pespin <pes...@sysmocom.de>
Gerrit-MessageType: merged
