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/.
neels gerrit-no-reply at lists.osmocom.orgneels 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 <nhofmeyr at sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels <nhofmeyr at sysmocom.de> Gerrit-Reviewer: osmith <osmith at sysmocom.de> 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/20190829/b89ce2ca/attachment.htm>