[asterisk-dev] [Code Review] 4590: Optional API does not work for sources that are both provider and user of optional API's.

[Joshua Colp]

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



/trunk/include/asterisk/optional_api.h
<https://reviewboard.asterisk.org/r/4590/#comment25796>

    Extend this comment to explain why so the next person looking at it will know what is up. It took me a bit after looking at things to understand what exactly was going on.


- Joshua Colp


On April 4, 2015, 12:17 p.m., Corey Farrell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4590/
> -----------------------------------------------------------
> 
> (Updated April 4, 2015, 12:17 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-17608
>     https://issues.asterisk.org/jira/browse/ASTERISK-17608
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> OPTIONAL_API has conditionals to define AST_OPTIONAL_API and AST_OPTIONAL_API_ATTR differently based on if AST_API_MODULE is defined.  Unfortunately this is inside the "#ifndef __ASTERISK_OPTIONAL_API_H" include protection block, so only the first status of AST_API_MODULE is respected.  For example res_monitor is a provider, but uses beep.h (func_periodic_hook).  This makes func_periodic_hook non-optional to res_monitor.
> 
> The patch applies cleanly to 13 and fixes the issue, but I'm not sure this should be fixed in 13.  Although there is no ABI change, it is a change in behaviour for sources that #include optional_api.h.  If this is wanted for 13 please let me know, otherwise I will follow up with a patch for 13 to make res_monitor require func_periodic_hook.
> 
> 
> Diffs
> -----
> 
>   /trunk/include/asterisk/optional_api.h 433966 
> 
> Diff: https://reviewboard.asterisk.org/r/4590/diff/
> 
> 
> Testing
> -------
> 
> Verified I could load res_monitor with or without func_periodic_hook.  Ran a couple testsuite tests.
> 
> 
> Thanks,
> 
> Corey Farrell
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20150408/d493358a/attachment-0001.html>