[asterisk-dev] [Code Review] Use va_copy for stringfields--it's used everywhere else

Russell Bryant reviewboard at asterisk.org
Thu May 26 10:26:11 CDT 2011


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

Ship it!


- Russell


On 2011-05-26 10:04:35, Terry Wilson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1233/
> -----------------------------------------------------------
> 
> (Updated 2011-05-26 10:04:35)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> The ast_string_field_build_va functions were written to take to separate va_lists to work around FreeBSD 4 not having va_copy defined. It should have been perfectly possible for that case to just define va_copy ourselves as an assignment, as if va_list was implemented in some way incompatible with that (as in an array type), passing two copies of it wouldn't have worked anyway.
> 
> In the end, we don't support anything using gcc < 3 anyway because we use va_copy all over the place anyway. This patch just simplifies things by removing the second va_list function arguments in favor of va_copy. The only place that used the function was __ast_channel_alloc_ap(). We could simplify things further by just getting rid of __ast_channel_alloc_ap() and doing everything in __ast_channel_alloc() since it is only called by __ast_channel_alloc().
> 
> Also, does the ABI compatibility stuff at the end of channel.c still need to be there in trunk?
> 
> 
> Diffs
> -----
> 
>   /trunk/include/asterisk/stringfields.h 320708 
>   /trunk/main/channel.c 320708 
>   /trunk/main/utils.c 320708 
> 
> Diff: https://reviewboard.asterisk.org/r/1233/diff
> 
> 
> Testing
> -------
> 
> Made a call under valgrind. Things didn't blow up and the channel had the appropriate name, so things must work.
> 
> 
> Thanks,
> 
> Terry
> 
>

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


More information about the asterisk-dev mailing list