[asterisk-dev] [Code Review] Allow empty display name if callerid is set to an empty string. Take 2!
rmudgett
reviewboard at asterisk.org
Wed Aug 3 10:40:51 CDT 2011
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1341/#review3981
-----------------------------------------------------------
Is there going to be a version for 1.8,...?
/trunk/channels/chan_sip.c
<https://reviewboard.asterisk.org/r/1341/#comment7829>
Move the from[] line to later as indicated.
/trunk/channels/chan_sip.c
<https://reviewboard.asterisk.org/r/1341/#comment7831>
Make code like:
ourport = ...
if (cid_has_name) {
/* Add a display name */
sprintf...
} else {
from[0] = '\0';
}
if (!sip_stand....
1) Code is grouped for the purpose of generating the from string.
2) The from[0] initialization is delayed until needed and you don't have to look for it.
/trunk/channels/chan_sip.c
<https://reviewboard.asterisk.org/r/1341/#comment7830>
Remember that strlen() takes time to execute and you are calling it 4 times here. Just save the strlen() value before the if and use it in the 4 places.
- rmudgett
On Aug. 3, 2011, 9:57 a.m., jrose wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1341/
> -----------------------------------------------------------
>
> (Updated Aug. 3, 2011, 9:57 a.m.)
>
>
> Review request for Asterisk Developers, David Vossel and rmudgett.
>
>
> Summary
> -------
>
> This is basically what I was trying to do with the user submitted patch from yesterday. After speaking with Richard, it became somewhat obvious that the approach to the patch had too much extra impact on the SIP messages when all that was really needed was specifically targeting the creation of the display name.
>
> This approach checks strictly for a caller id name and if it isn't there, we skip the creation of the display name. I think this should be safer in general than the previous approach.
>
>
> This addresses bug ASTERISK-16198.
> https://issues.asterisk.org/jira/browse/ASTERISK-16198
>
>
> Diffs
> -----
>
> /trunk/CHANGES 330571
> /trunk/channels/chan_sip.c 330571
>
> Diff: https://reviewboard.asterisk.org/r/1341/diff
>
>
> Testing
> -------
>
> Testing was similar to last time with focus on comparing the SIP messages between this branch and the normal trunk. In this case, only the display name gets changed.
>
> A few SIP tests from the test suite were also ran successfully. I haven't ran the full test suite yet, but am preparing to do so.
>
>
> Thanks,
>
> jrose
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20110803/9281a53f/attachment-0001.htm>
More information about the asterisk-dev
mailing list