<p>neels <strong>merged</strong> this change.</p><p><a href="https://gerrit.osmocom.org/c/osmo-mgw/+/15140">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Jenkins Builder: Verified
osmith: Looks good to me, approved
pespin: Looks good to me, but someone else must approve
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">tweak mgcp_parse_audio_ptime_rtpmap()<br><br>- move the error logging up to the actual errors. Each appear only once, no<br> goto labels needed.<br><br>- instead of strstr("rtpmap"), use osmo_str_startswith("a=rtpmap:") to more<br> concisely trigger on the actual syntax of the audio parameters. Same for<br> "a=ptime:".<br><br>Change-Id: I730111e245da8485c1b5e8811f75d140e379cec6<br>---<br>M src/libosmo-mgcp-client/mgcp_client.c<br>1 file changed, 33 insertions(+), 35 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/libosmo-mgcp-client/mgcp_client.c b/src/libosmo-mgcp-client/mgcp_client.c</span><br><span>index 3f8e780..6275385 100644</span><br><span>--- a/src/libosmo-mgcp-client/mgcp_client.c</span><br><span>+++ b/src/libosmo-mgcp-client/mgcp_client.c</span><br><span>@@ -339,46 +339,44 @@</span><br><span> char codec_resp[64];</span><br><span> unsigned int codec;</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+#define A_PTIME "a=ptime:"</span><br><span style="color: hsl(120, 100%, 40%);">+#define A_RTPMAP "a=rtpmap:"</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- if (strstr(line, "ptime")) {</span><br><span style="color: hsl(0, 100%, 40%);">- if (sscanf(line, "a=ptime:%u", &r->ptime) != 1)</span><br><span style="color: hsl(0, 100%, 40%);">- goto response_parse_failure_ptime;</span><br><span style="color: hsl(0, 100%, 40%);">- } else if (strstr(line, "rtpmap")) {</span><br><span style="color: hsl(0, 100%, 40%);">- if (sscanf(line, "a=rtpmap:%d %63s", &pt, codec_resp) == 2) {</span><br><span style="color: hsl(0, 100%, 40%);">- /* The MGW may assign an own payload type in the</span><br><span style="color: hsl(0, 100%, 40%);">- * response if the choosen codec falls into the IANA</span><br><span style="color: hsl(0, 100%, 40%);">- * assigned dynamic payload type range (96-127).</span><br><span style="color: hsl(0, 100%, 40%);">- * Normally the MGW should obey the 3gpp payload type</span><br><span style="color: hsl(0, 100%, 40%);">- * assignments, which are fixed, so we likely wont see</span><br><span style="color: hsl(0, 100%, 40%);">- * anything unexpected here. In order to be sure that</span><br><span style="color: hsl(0, 100%, 40%);">- * we will now check the codec string and if the result</span><br><span style="color: hsl(0, 100%, 40%);">- * does not match to what is IANA / 3gpp assigned, we</span><br><span style="color: hsl(0, 100%, 40%);">- * will create an entry in the ptmap table so we can</span><br><span style="color: hsl(0, 100%, 40%);">- * lookup later what has been assigned. */</span><br><span style="color: hsl(0, 100%, 40%);">- codec = map_str_to_codec(codec_resp);</span><br><span style="color: hsl(0, 100%, 40%);">- if (codec != pt) {</span><br><span style="color: hsl(0, 100%, 40%);">- if (r->ptmap_len < ARRAY_SIZE(r->ptmap)) {</span><br><span style="color: hsl(0, 100%, 40%);">- r->ptmap[r->ptmap_len].pt = pt;</span><br><span style="color: hsl(0, 100%, 40%);">- r->ptmap[r->ptmap_len].codec = codec;</span><br><span style="color: hsl(0, 100%, 40%);">- r->ptmap_len++;</span><br><span style="color: hsl(0, 100%, 40%);">- } else</span><br><span style="color: hsl(0, 100%, 40%);">- goto response_parse_failure_rtpmap;</span><br><span style="color: hsl(120, 100%, 40%);">+ if (osmo_str_startswith(line, A_PTIME)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (sscanf(line, A_PTIME "%u", &r->ptime) != 1) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DLMGCP, LOGL_ERROR,</span><br><span style="color: hsl(120, 100%, 40%);">+ "Failed to parse SDP parameter, invalid ptime (%s)\n", line);</span><br><span style="color: hsl(120, 100%, 40%);">+ return -EINVAL;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ } else if (osmo_str_startswith(line, A_RTPMAP)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (sscanf(line, A_RTPMAP "%d %63s", &pt, codec_resp) != 2) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DLMGCP, LOGL_ERROR,</span><br><span style="color: hsl(120, 100%, 40%);">+ "Failed to parse SDP parameter, invalid rtpmap: %s\n", osmo_quote_str(line, -1));</span><br><span style="color: hsl(120, 100%, 40%);">+ return -EINVAL;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ /* The MGW may assign an own payload type in the</span><br><span style="color: hsl(120, 100%, 40%);">+ * response if the choosen codec falls into the IANA</span><br><span style="color: hsl(120, 100%, 40%);">+ * assigned dynamic payload type range (96-127).</span><br><span style="color: hsl(120, 100%, 40%);">+ * Normally the MGW should obey the 3gpp payload type</span><br><span style="color: hsl(120, 100%, 40%);">+ * assignments, which are fixed, so we likely wont see</span><br><span style="color: hsl(120, 100%, 40%);">+ * anything unexpected here. In order to be sure that</span><br><span style="color: hsl(120, 100%, 40%);">+ * we will now check the codec string and if the result</span><br><span style="color: hsl(120, 100%, 40%);">+ * does not match to what is IANA / 3gpp assigned, we</span><br><span style="color: hsl(120, 100%, 40%);">+ * will create an entry in the ptmap table so we can</span><br><span style="color: hsl(120, 100%, 40%);">+ * lookup later what has been assigned. */</span><br><span style="color: hsl(120, 100%, 40%);">+ codec = map_str_to_codec(codec_resp);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (codec != pt) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (r->ptmap_len >= ARRAY_SIZE(r->ptmap)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DLMGCP, LOGL_ERROR, "No more space in ptmap array (len=%u)\n", r->ptmap_len);</span><br><span style="color: hsl(120, 100%, 40%);">+ return -ENOSPC;</span><br><span> }</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- } else</span><br><span style="color: hsl(0, 100%, 40%);">- goto response_parse_failure_rtpmap;</span><br><span style="color: hsl(120, 100%, 40%);">+ r->ptmap[r->ptmap_len].pt = pt;</span><br><span style="color: hsl(120, 100%, 40%);">+ r->ptmap[r->ptmap_len].codec = codec;</span><br><span style="color: hsl(120, 100%, 40%);">+ r->ptmap_len++;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span> }</span><br><span> </span><br><span> return 0;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">-response_parse_failure_ptime:</span><br><span style="color: hsl(0, 100%, 40%);">- LOGP(DLMGCP, LOGL_ERROR,</span><br><span style="color: hsl(0, 100%, 40%);">- "Failed to parse SDP parameter, invalid ptime (%s)\n", line);</span><br><span style="color: hsl(0, 100%, 40%);">- return -EINVAL;</span><br><span style="color: hsl(0, 100%, 40%);">-response_parse_failure_rtpmap:</span><br><span style="color: hsl(0, 100%, 40%);">- LOGP(DLMGCP, LOGL_ERROR,</span><br><span style="color: hsl(0, 100%, 40%);">- "Failed to parse SDP parameter, invalid rtpmap (%s)\n", line);</span><br><span style="color: hsl(0, 100%, 40%);">- return -EINVAL;</span><br><span> }</span><br><span> </span><br><span> /* Parse a line like "c=IN IP4 10.11.12.13" */</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-mgw/+/15140">change 15140</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/c/osmo-mgw/+/15140"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: osmo-mgw </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I730111e245da8485c1b5e8811f75d140e379cec6 </div>
<div style="display:none"> Gerrit-Change-Number: 15140 </div>
<div style="display:none"> Gerrit-PatchSet: 5 </div>
<div style="display:none"> Gerrit-Owner: neels <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: neels <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: osmith <osmith@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>