[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