[asterisk-dev] [Code Review] Improve SDP parsing warning messages and RFC compliance

Kevin Fleming reviewboard at asterisk.org
Fri Mar 23 15:34:17 CDT 2012


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1811/
-----------------------------------------------------------

(Updated March 23, 2012, 3:34 p.m.)


Review request for Asterisk Developers and opticron.


Changes
-------

Updated to remove most uses of 'continue' in the m-line processing loop, along with correcting some comments, removing some excess whitespace, and making process_sdp() return -1 for all failures (none of its callers did anything with the various different values it could return, and they were undocumented). If there is value in the future in having process_sdp() return a different error code for different problems found in the SDP, an enum can be added with those error codes so they can be given symbolic names.


Summary (updated)
-------

This patch improves the warning messages generated during SDP parsing when various problems are found:

* 'Unsupported media type' is only reported when that is in fact the case, not when a supported media type is included in an 'm' line that has an invalid format.

* All warning messages related to parsing 'm' lines now include the 'm' line contents.

* (minor bugfix) newline added to port-number-zero warning messages.

* Warning messages improved to use RFC-specified terminology for various items.

* Warnings for offers that include more than port for a single media type now include the media type.

* Nearly all SDP errors will result in a '488 Not Acceptable Here' response, rather than an attempt to use some part of the offer/answer presented in the SDP.

This patch does *not* correct Asterisk's behavior when a media stream offer with a port number of 0 (zero) is received; that will have to be done in another patch.


Diffs (updated)
-----

  /branches/1.8/channels/chan_sip.c 360261 

Diff: https://reviewboard.asterisk.org/r/1811/diff


Testing
-------

Compile testing only.


Thanks,

Kevin

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120323/9d367a64/attachment.htm>


More information about the asterisk-dev mailing list