[asterisk-dev] [Code Review] Compare T.38 attributes in a case insensitive manner

[Matt Jordan]

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

Review request for Asterisk Developers and wdoekes.


Summary
-------

RFC5347 section 2.5.2 states the following:
...
The attribute "T38MaxBitRate" was once incorrectly registered with
IANA as "T38maxBitRate" (lower-case "m"). In accordance with T.38
examples and common implementation practice, the form "T38MaxBitRate"
SHOULD be generated by implementations conforming to this package.
In general, it is RECOMMENDED that implementations of this package
accept lowercase, uppercase, and mixed upper/lowercase encodings of
all the T.38 attributes.
...

Asterisk currently does not perform case insensitive matching on the T.38 attributes. This causes the T38MaxBitRate attribute to be negotiated at 2400 baud instead of 14400 (or whatever value you actually wanted).

Given that we use sscanf, there's two ways to go about solving this:

* Convert the received attribute to lower/upper case and compare that way (this is what this patch does)
* Use sscanf to match both lower/upper case letters for every letter in the attribute, i.e., sscanf(a, "%*[tT]38%*[mM]%*[aA]%*[xX]%*[bB]%*[iI]%*[tT]%*[rR]%*[aA]%*[tT]%*[eE]:%30u", &x)

I went with the all lower option as it's a lot easier to read. The sscanf option is a bit more performant, as you don't have to convert the attribute to another case. (Note that as Walter suggested on ASTERISK-20897, we could write our own tolower method that did this a bit faster, but that feels like pre-mature optimization in this case).

If folks feel strongly about using a slightly faster approach, I'm open to that.


This addresses bug ASTERISK-20897.
    https://issues.asterisk.org/jira/browse/ASTERISK-20897


Diffs
-----

  /branches/1.8/channels/chan_sip.c 380332 

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


Testing
-------


Thanks,

Matt

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20130129/6f6b43f6/attachment-0001.htm>