[asterisk-dev] [Code Review] va_args cleanup in res_config_odbc, res_config_curl, utils, and also res_config_pgsql

wdoekes reviewboard at asterisk.org
Tue Apr 17 15:07:24 CDT 2012


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

Ship it!


Looks good to me.

Apart from addons/res_config_mysql.c, I think you got them all.

- wdoekes


On April 6, 2012, 8:47 a.m., jrose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1848/
> -----------------------------------------------------------
> 
> (Updated April 6, 2012, 8:47 a.m.)
> 
> 
> Review request for Asterisk Developers, Mark Michelson, wdoekes, and Matt Jordan.
> 
> 
> Summary
> -------
> 
> A number of va_copy operations weren't matched with a corresponding va_end in res_config_odbc. Also, there was a potential for va_end to be invoked twice on the same va_arg in utils, which would mean invoking va_end on an undefined variable... which is bad.
> 
> va_end is removed from various functions in config_pgsql and config_curl since they aren't making their own copy.  The invokers of those functions are responsible for calling va_end on them.
> 
> 
> This addresses bug ASTERISK-19451.
>     https://issues.asterisk.org/jira/browse/ASTERISK-19451
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/main/utils.c 361291 
>   /branches/1.8/res/res_config_curl.c 361291 
>   /branches/1.8/res/res_config_odbc.c 361291 
>   /branches/1.8/res/res_config_pgsql.c 361291 
> 
> Diff: https://reviewboard.asterisk.org/r/1848/diff
> 
> 
> Testing
> -------
> 
> None to speak of.  It compiles, but I'm really not sure how to check for va_arg leaks and such.  I'm not too worried about res_config_odbc and utils since those changes are pretty self-contained. The other changes should be fine too since the paramater ap in res_config_odbc isn't ended in any of the adapter functions there. I'm a little wary though that they should be copying to a new va_arg list instead like res_config_odbc does.
> 
> 
> Thanks,
> 
> jrose
> 
>

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


More information about the asterisk-dev mailing list