[asterisk-dev] [Code Review] Use new ao2_global_obj API to clean up udptl.c config parsing/reloading

[Matt Jordan]

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



/trunk/main/udptl.c
<https://reviewboard.asterisk.org/r/1840/#comment10901>

    This is a bit picky and possibly not worth the effort, but the debug settings should theoretically probably be contained in udptl_global_options as well.
    
    When a CLI command is used to turn debug on/off, its operating on a separate thread then whatever is going to actually use the debug flags.  This puts us in a condition where the udptldebug flag could be enabled, but we have not yet fully initialized udptldebugaddr.
    
    One way of doing this would be to obtain a reference to the current config object in the CLI command, lock it, update the values, and unlock it.  Since these values change on a thread other then a reload, we'd have to lock the config object anytime we wanted to read values from it as well (or at least the debug values).
    
    Alternatively, we could do a clone and swap - grab the config object in the CLI command, make a copy of it, populate it with the new debug values, and swap it with the new one.  That'd be a lot less locking and less impact on the rest of the code.



/trunk/main/udptl.c
<https://reviewboard.asterisk.org/r/1840/#comment10900>

    I'd go ahead and doxygen comment what each of these fields actually are.  "Start" and "End" are a bit generic


- Matt


On April 2, 2012, 2:06 p.m., Terry Wilson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1840/
> -----------------------------------------------------------
> 
> (Updated April 2, 2012, 2:06 p.m.)
> 
> 
> Review request for Asterisk Developers and Matt Jordan.
> 
> 
> Summary
> -------
> 
> This patch builds off of https://reviewboard.asterisk.org/r/1455/.
> 
> The differences are:
> 1) Uses the global ao2 object API to store config values and to make reloads thread-safe
> 2) Uses ast_parse_arg() where appropriate
> 3) Updates ast_parse_arg to be able to set defaults to the min/max range value on range errors
> 4) Adds CLI command 'udptl show config'
> 
> 
> Diffs
> -----
> 
>   /trunk/include/asterisk/config.h 360980 
>   /trunk/main/config.c 360980 
>   /trunk/main/udptl.c 360980 
> 
> Diff: https://reviewboard.asterisk.org/r/1840/diff
> 
> 
> Testing
> -------
> 
> 1) Used the new 'udptl show config' command to verify that settings looked right on load/reload.
> 2) Did testing with 'memory show summary config.c/udptl.c' with loads/reloads to verify there are no leaks
> 3) Loaded with no config file to verify that default values were used
> 
> 
> Thanks,
> 
> Terry
> 
>

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