[asterisk-dev] [Code Review] ADSI and Crypto backwards compatibility with 1.6.2

[Kevin Fleming]

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



/branches/1.8/apps/app_adsiprog.c
<https://reviewboard.asterisk.org/r/876/#comment5751>

    In all of these cases, there doesn't seem to be any harm in populating the required_resource field in every build, even if that particular system won't use it. I'd honestly prefer a solution that was derived directly from the MODINFO blocks, so that it couldn't get out of sync, but I understand that would be quite complex to build.



/branches/1.8/apps/app_followme.c
<https://reviewboard.asterisk.org/r/876/#comment5752>

    Unrelated change.



/branches/1.8/apps/app_queue.c
<https://reviewboard.asterisk.org/r/876/#comment5753>

    Was this missed in the previous conversion? It doesn't look like this new code makes the dependency less strong... the previous changes to optional_api did that.



/branches/1.8/channels/chan_mgcp.c
<https://reviewboard.asterisk.org/r/876/#comment5754>

    Is this true? Does res_pktccops use optional_api? If not, then chan_mgcp shouldn't be buildable if res_pktccops isn't built as well. Delaying the problem until runtime (as a loading error) doesn't really help the system administrator much.



/branches/1.8/include/asterisk/module.h
<https://reviewboard.asterisk.org/r/876/#comment5755>

    Make this field already exist; changing the composition of an API structure based on platform dependencies seems very unsafe.



/branches/1.8/main/loader.c
<https://reviewboard.asterisk.org/r/876/#comment5756>

    What will happen if loading the dependency module fails here?



/branches/1.8/pbx/pbx_loopback.c
<https://reviewboard.asterisk.org/r/876/#comment5757>

    Unrelated change.



/branches/1.8/pbx/pbx_realtime.c
<https://reviewboard.asterisk.org/r/876/#comment5758>

    Unrelated change.


- Kevin


On 2010-08-24 21:02:08, Tilghman Lesher wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/876/
> -----------------------------------------------------------
> 
> (Updated 2010-08-24 21:02:08)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> In 1.6.2 and previous, res_adsi and res_crypto used stub files to allow them not to be loaded.  We got rid of the stubs in favor of the optional_api approach.  Now people who are upgrading, who previously noload'ed those modules AND are on a platform which does not support optional_api (i.e. gcc 4.1) (e.g. RHEL, Centos 5) are complaining that the easy upgrade path is broken.
> 
> This patch forces those broken platforms to automatically load the dependencies, when required by a loaded module.
> 
> 
> This addresses bug 17707.
>     https://issues.asterisk.org/view.php?id=17707
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/apps/app_adsiprog.c 283524 
>   /branches/1.8/apps/app_followme.c 283524 
>   /branches/1.8/apps/app_getcpeid.c 283524 
>   /branches/1.8/apps/app_queue.c 283524 
>   /branches/1.8/apps/app_speech_utils.c 283524 
>   /branches/1.8/apps/app_stack.c 283524 
>   /branches/1.8/apps/app_voicemail.c 283524 
>   /branches/1.8/channels/chan_agent.c 283524 
>   /branches/1.8/channels/chan_dahdi.c 283524 
>   /branches/1.8/channels/chan_iax2.c 283524 
>   /branches/1.8/channels/chan_mgcp.c 283524 
>   /branches/1.8/channels/chan_sip.c 283524 
>   /branches/1.8/funcs/func_aes.c 283524 
>   /branches/1.8/include/asterisk/module.h 283524 
>   /branches/1.8/main/loader.c 283524 
>   /branches/1.8/pbx/pbx_dundi.c 283524 
>   /branches/1.8/pbx/pbx_loopback.c 283524 
>   /branches/1.8/pbx/pbx_realtime.c 283524 
> 
> Diff: https://reviewboard.asterisk.org/r/876/diff
> 
> 
> Testing
> -------
> 
> noloaded modules on a Centos 5 machine, verified they loaded when a dependent module was loaded, either at boot time, or later, at CLI request (i.e. the 'module load' command).
> 
> 
> Thanks,
> 
> Tilghman
> 
>