[asterisk-dev] [Code Review]: Fix a few leftover non-opaque ast_str uses

[Russell Bryant]

> On Nov. 16, 2012, 9:21 a.m., elguero wrote:
> > /branches/1.8/channels/chan_sip.c, line 6667
> > <https://reviewboard.asterisk.org/r/2198/diff/1/?file=32168#file32168line6667>
> >
> >     Curious about this change.  I think the code is just a tiny, tad bit, clearer with ast_strlen_zero.

Personally I find !strlen() just as readable as strlen_zero().

The origin of ast_strlen_zero() is that it's much more efficient to only check the first byte if all care about is if the string is empty or not.  With ast_str, ast_str_strlen() is a constant time operation since the current length is stored in the ast_str struct.

The point of this particular change was to get rid of non-opaque usage of ast_str (reaching for ->str directly).  To do that and still use ast_strlen_zero, it would look like:

  !ast_strlen_zero(ast_str_buffer(p->initreq.data))

which is much more horrendous than:

  ast_str_strlen(p->initreq.data)


- Russell


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


On Nov. 16, 2012, 6:35 a.m., wdoekes wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2198/
> -----------------------------------------------------------
> 
> (Updated Nov. 16, 2012, 6:35 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> This patch takes care of some low hanging str->str => ast_str_buffer(str) fruit.
> 
> According to my compiler, there are a few non-opaque ast_str string uses left after this (only in chan_sip), but they can be taken care of some other time.
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/channels/chan_sip.c 376383 
>   /branches/1.8/main/indications.c 376383 
>   /branches/1.8/main/security_events.c 376383 
> 
> Diff: https://reviewboard.asterisk.org/r/2198/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> wdoekes
> 
>

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