[asterisk-dev] [Code Review] Correct ast_api_register/unregister_multiple() API calls

[Russell Bryant]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/59/#review145
-----------------------------------------------------------

Ship it!


Other than one little nitpick, this looks good to go.


/trunk/res/res_agi.c
<http://reviewboard.digium.com/r/59/#comment265>

    You could reduce indentation here by doing:
    
    if (ast_agi_register(...) == 1) {
        x++;
        continue;
    }
    
    /* Error handling */


- Russell


On 2008-11-18 11:43:13, Kevin Fleming wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.digium.com/r/59/
> -----------------------------------------------------------
> 
> (Updated 2008-11-18 11:43:13)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> Revision 77787 (which came from issue 10288 in Mantis) introduced a number of API changes in res_agi.c and agi.h. Included among these was the addition of ast_agi_register_multiple() and ast_agi_unregister_multiple(), which were supposedly clones of the ast_cli_entry variants.
> 
> However, neither of these functions actually checked for failures to register or unregister the individual entries they process, and they did not have the ability to report errors back to the caller either.
> 
> This patch would change these API calls to properly check for and return errors, and fallback in as graceful a way as can be done when failures occur. It also adds Doxygen documentation for them, something which was also lacking in the original revision but it supposed to be included whenever any new API call is added.
> 
> Changing this will be an API change in the 1.6 release series, but the API is broken and must be fixed. In this patch I have renamed UPGRADE.txt to UPGRADE-1.6.txt and started a new UPGRADE.txt; for trunk and 1.6.1, this will document changes *between* 1.6 releases that users must be aware of, and of course this API change will be documented there in those two branches. In the 1.6.0 branch, I could either do the same thing (so only 1.6.0 and 1.6.0.1 would have UPGRADE.txt being relative to 1.4, and future 1.6.0.x releases would be relative to previous releases), or add a special note to the existing UPGRADE.txt file. Comments welcome :-)
> 
> 
> Diffs
> -----
> 
>   /trunk/UPGRADE.txt 157301 
>   /trunk/apps/app_stack.c 157301 
>   /trunk/include/asterisk/agi.h 157301 
>   /trunk/res/res_agi.c 157301 
> 
> Diff: http://reviewboard.digium.com/r/59/diff
> 
> 
> Testing
> -------
> 
> Compile testing.
> 
> 
> Thanks,
> 
> Kevin
> 
>