[asterisk-dev] [Code Review]: Fix a few leftover non-opaque ast_str uses
elguero
reviewboard at asterisk.org
Fri Nov 16 09:32:32 CST 2012
> 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.
>
> Russell Bryant wrote:
> 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)
Yep, thanks for the extra info to help me see things clearer :)
After I made my comments, it clicked in my head that this had to do with the non-opaque usage...
- elguero
-----------------------------------------------------------
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/fa3096bc/attachment.htm>
More information about the asterisk-dev
mailing list