[asterisk-dev] [Code Review] 3201: realtime: Fix ast_update2_realtime() on raspberry pi.

[Mark Michelson]

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

Ship it!


Ship It!

- Mark Michelson


On Feb. 9, 2014, 9:44 a.m., wdoekes wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3201/
> -----------------------------------------------------------
> 
> (Updated Feb. 9, 2014, 9:44 a.m.)
> 
> 
> Review request for Asterisk Developers and Julian Lyndon-Smith.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> jmls got the following odd query on his raspberry PI:
> 
>   [Feb  7 08:36:59] ERROR[3516][C-00000029]: res_config_pgsql.c:167 _pgsql_exec:
>   PostgreSQL RealTime: Query Failed:
> 
>   UPDATE h42_voicemail SET context='f5f985...', mailbox='502'
>   WHERE                    context='f5f985...' AND mailbox='502'
> 
> Notice how that is essentially a no-op.
> 
> The cause turned out to be this asterisk 12 code in main/config.c:
> 
>   va_start(ap);
>   lookup_fields = realtime_arguments_to_fields(ap);
>   update_fields = realtime_arguments_to_fields(ap);
>   va_end(ap);
> 
> According to the va_arg manpage, the behaviour of 'ap' after function return is undefined.
> In the raspberry case, it was reset, so jmls would get the "lookup fields" twice and never
> the "update fields" (the password).
> 
> 
> This patch:
> 
> - Surrounds both realtime_argument_to_fields calls with a matching va_start+va_end (legal,
>   according to man).
> - Adds a skip parameter to realtime_arguments_to_fields2(ap, skip), so we can skip past the
>   first arguments on the second call.
> - Adds a bunch of early returns for cases when ast_variable_new fails (out of mem?) so the
>   realtime implementations won't have to crash.
> 
> 
> Diffs
> -----
> 
>   /branches/12/main/config.c 406079 
> 
> Diff: https://reviewboard.asterisk.org/r/3201/diff/
> 
> 
> Testing
> -------
> 
> It compiles.
> 
> jmls confirmed that the second call using 'ap' yielded the first result set, instead of the expected second.
> 
> jmls confirmed that the patch fixes his problem.
> 
>   10:14 <jmls> meh - doesn't look like I have access to the reviewboard ..
>   10:38 <jmls> but can confirm that the password update now works just fine
> 
> 
> Thanks,
> 
> wdoekes
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140211/844e0c34/attachment.html>