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

[Corey Farrell]

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

(Updated April 8, 2015, 2:21 p.m.)


Review request for Asterisk Developers.


Changes
-------

Improve comment about section outside the include guard.

Moved "#undef AST_API_MODULE" to happen even when OPTIONAL_API is disabled.  In main/utils.c AST_API_MODULE is defined before multiple headers, I've seen compilers complain about #define on an already defined macro.


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 (updated)
-----

  /trunk/include/asterisk/optional_api.h 434423 

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/b0be27e8/attachment-0001.html>