[asterisk-dev] [Code Review] CLI command 'manager show settings'

[paul.belanger at polybeacon.com]

> On 2010-04-27 19:29:30, pabelanger wrote:
> > /trunk/main/manager.c, line 5734
> > <https://reviewboard.asterisk.org/r/630/diff/1/?file=9651#file9651line5734>
> >
> >     I'm under the impression this may generate a memory leak, however I'm not sure how to handle this.
> 
> Mark Michelson wrote:
>     First off, you are correct that it may cause a memory leak. Every time the manager is reloaded, you will leak memory. There are two ways to handle this:
>     
>     1. Use ast_free to free manager_channelvars before using ast_strdup. You'd have to modify a few other places if you did this. For instance, you'd have to change the global default value for manager_channelvars from "" to NULL. Then, since manager_channelvars has the potential to be NULL, you'd also have to modify the manager show settings command to possibly handle a NULL value for manager_channelvars (easy to do with the S_OR macro).
>     2. Use a statically sized buffer for manager_channelvars and use ast_copy_string to place the value in instead. This has the advantage of being easier than the other methd, but it could result in some wasted memory if the buffer is significantly larger than the actual amount of data being placed in it.

Fixed.  Went with option 1.  I believe I followed your instructions correctly.


- pabelanger


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


On 2010-04-28 10:55:22, pabelanger wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/630/
> -----------------------------------------------------------
> 
> (Updated 2010-04-28 10:55:22)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> This patch adds 'manager show settings' CLI command.  Most of the logic was used from chan_sip.c.
> 
> 
> This addresses bug 16343.
>     https://issues.asterisk.org/view.php?id=16343
> 
> 
> Diffs
> -----
> 
>   /trunk/channels/chan_sip.c 259663 
>   /trunk/include/asterisk/cli.h 259663 
>   /trunk/include/asterisk/manager.h 259663 
>   /trunk/main/manager.c 259663 
> 
> Diff: https://reviewboard.asterisk.org/r/630/diff
> 
> 
> Testing
> -------
> 
> Local developers and testing boxes.
> 
> 
> Thanks,
> 
> pabelanger
> 
>