[asterisk-dev] [Code Review] Opaquify ast_channel lists and structs

[Tilghman Lesher]

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



/trunk/main/channel_internal_api.c
<https://reviewboard.asterisk.org/r/1773/#comment10451>

    Hopefully, this API is going to be revisited at some point before a major release is made.  There's always been some discussion about the inefficiency of a linked list for variables, and this change, although making channel opaque, does not actually allow the backend of channel variable storage to be changed in any real way.
    
    It needs additional accessor functions, e.g. ast_channel_variable_name(), ast_channel_variable_value(), ast_channel_variable_attribute(), ast_channel_variable_lookup_next(), etc., with an opaque variable container, so that the implementation can change to e.g. a hash table backend.
    
    My concern is that while we're going through the pain of changing the core channel API, we should be ensuring this API  actually makes the storage of these values really opaque, instead of just hiding a pointer, as this is now.


- Tilghman


On Feb. 27, 2012, 5:47 p.m., Terry Wilson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1773/
> -----------------------------------------------------------
> 
> (Updated Feb. 27, 2012, 5:47 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> Opaquify lists and remaining structs. I had do a lot of manual fixing in this one. I also added a SWAP macro in chan_local because I was tired of typing and saw some copy/paste code that could use it. I can type things out if people really want me to. :-p Setters for the structs are just wrappers around memcpy. For the most part I could have done things like *ast_channel_caller(chan1) = *ast_channel_caller(chan2), but ick.
> 
> 
> Diffs
> -----
> 
>   /trunk/addons/chan_ooh323.c 357092 
>   /trunk/apps/app_alarmreceiver.c 357092 
>   /trunk/apps/app_amd.c 357092 
>   /trunk/apps/app_confbridge.c 357092 
>   /trunk/apps/app_dial.c 357092 
>   /trunk/apps/app_disa.c 357092 
>   /trunk/apps/app_dumpchan.c 357092 
>   /trunk/apps/app_fax.c 357092 
>   /trunk/apps/app_followme.c 357092 
>   /trunk/apps/app_macro.c 357092 
>   /trunk/apps/app_meetme.c 357092 
>   /trunk/apps/app_minivm.c 357092 
>   /trunk/apps/app_mixmonitor.c 357092 
>   /trunk/apps/app_osplookup.c 357092 
>   /trunk/apps/app_parkandannounce.c 357092 
>   /trunk/apps/app_privacy.c 357092 
>   /trunk/apps/app_queue.c 357092 
>   /trunk/apps/app_readexten.c 357092 
>   /trunk/apps/app_rpt.c 357092 
>   /trunk/apps/app_setcallerid.c 357092 
>   /trunk/apps/app_sms.c 357092 
>   /trunk/apps/app_stack.c 357092 
>   /trunk/apps/app_talkdetect.c 357092 
>   /trunk/apps/app_voicemail.c 357092 
>   /trunk/apps/app_while.c 357092 
>   /trunk/apps/app_zapateller.c 357092 
>   /trunk/bridges/bridge_builtin_features.c 357092 
>   /trunk/channels/chan_agent.c 357092 
>   /trunk/channels/chan_console.c 357092 
>   /trunk/channels/chan_dahdi.c 357092 
>   /trunk/channels/chan_gtalk.c 357092 
>   /trunk/channels/chan_h323.c 357092 
>   /trunk/channels/chan_iax2.c 357092 
>   /trunk/channels/chan_jingle.c 357092 
>   /trunk/channels/chan_local.c 357092 
>   /trunk/channels/chan_mgcp.c 357092 
>   /trunk/channels/chan_misdn.c 357092 
>   /trunk/channels/chan_oss.c 357092 
>   /trunk/channels/chan_phone.c 357092 
>   /trunk/channels/chan_sip.c 357092 
>   /trunk/channels/chan_skinny.c 357092 
>   /trunk/channels/chan_unistim.c 357092 
>   /trunk/channels/chan_usbradio.c 357092 
>   /trunk/channels/chan_vpb.cc 357092 
>   /trunk/channels/console_video.c 357092 
>   /trunk/channels/sig_analog.c 357092 
>   /trunk/channels/sig_pri.c 357092 
>   /trunk/channels/sig_ss7.c 357092 
>   /trunk/funcs/func_blacklist.c 357092 
>   /trunk/funcs/func_callerid.c 357092 
>   /trunk/funcs/func_dialplan.c 357092 
>   /trunk/funcs/func_strings.c 357092 
>   /trunk/funcs/func_timeout.c 357092 
>   /trunk/include/asterisk/channel.h 357092 
>   /trunk/include/asterisk/utils.h 357092 
>   /trunk/main/abstract_jb.c 357092 
>   /trunk/main/app.c 357092 
>   /trunk/main/autochan.c 357092 
>   /trunk/main/ccss.c 357092 
>   /trunk/main/cdr.c 357092 
>   /trunk/main/cel.c 357092 
>   /trunk/main/channel.c 357092 
>   /trunk/main/channel_internal_api.c 357092 
>   /trunk/main/cli.c 357092 
>   /trunk/main/dial.c 357092 
>   /trunk/main/features.c 357092 
>   /trunk/main/file.c 357092 
>   /trunk/main/manager.c 357092 
>   /trunk/main/message.c 357092 
>   /trunk/main/pbx.c 357092 
>   /trunk/pbx/pbx_lua.c 357092 
>   /trunk/res/res_agi.c 357092 
>   /trunk/res/res_fax.c 357092 
>   /trunk/res/snmp/agent.c 357092 
>   /trunk/tests/test_substitution.c 357092 
> 
> Diff: https://reviewboard.asterisk.org/r/1773/diff
> 
> 
> Testing
> -------
> 
> Unit tests pass.
> 
> 
> Thanks,
> 
> Terry
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120301/0c7c123e/attachment-0001.htm>