[asterisk-dev] [Code Review]: Allow empty display name if callerid is set to an empty string. Take 2!

[Kevin P. Fleming]

On 08/03/2011 11:02 AM, jrose wrote:

> Well, there are 4 calls, but it will only be called twice in any execution of the function.  Also keep in mind that only the display name can be entered into the from string at this point, so realistically we are talking like 30 characters on a bad day (and 0 characters in the case where we didn't have a caller id name) in a function that takes O(n) time based on length of the string that is going to be ran on a typical sip call once or twice at the beginning... I was thinking about it at the time, but it was at that level uncomfortably low impact that I almost thought it would be worse to have another variable floating around.
>
> That said, I do hate running functions twice in one statement, so I'll go ahead and use temp variables, but I think it would be better to have them before each snprintf statement than before the condition since they shouldn't have function-wide scope.

The compiler is going to optimize out the multiple calls to strlen on 
with the same argument; it knows that function is 'const' and doesn't 
cause any changes in its argument.

If you are going to change the way that the string is assembled in the 
buffer, I'd much rather see a change to using ast_str instead of 
reimplementing what it does for this one specific case; using snprintf() 
can result in unterminated strings if it's not done properly (and it's 
not being done properly here).

-- 
Kevin P. Fleming
Digium, Inc. | Director of Software Technologies
Jabber: kfleming at digium.com | SIP: kpfleming at digium.com | Skype: kpfleming
445 Jan Davis Drive NW - Huntsville, AL 35806 - USA
Check us out at www.digium.com & www.asterisk.org