[asterisk-dev] [Code Review] CLI filtering [GSoC 2009]

[Mark Michelson]

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


I made a second pass-through and have some more comments.


/trunk/main/logger.c
<https://reviewboard.asterisk.org/r/336/#comment2424>

    "Out of memory" warnings like this aren't necessary since ast_calloc will print a similar warning message itself.



/trunk/main/logger.c
<https://reviewboard.asterisk.org/r/336/#comment2425>

    This function needs some work.
    
    First, the "data" parameter is not a logger_filter_list_item, it is the filter_channels list for this thread. You need to traverse the list and free all allocated filters here.



/trunk/main/logger.c
<https://reviewboard.asterisk.org/r/336/#comment2426>

    The memory allocation used here seems excessive to me. I think that comparator and comparee should be statically-sized buffers (maybe either 128 or 256 bytes). Then, just use ast_copy_string to place the appropriate strings into the buffers.



/trunk/main/pbx.c
<https://reviewboard.asterisk.org/r/336/#comment2427>

    This function needs to be removed before being merged to trunk.



/trunk/main/pbx.c
<https://reviewboard.asterisk.org/r/336/#comment2428>

    Revert this change.


- Mark


On 2009-08-06 22:01:42, junky wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/336/
> -----------------------------------------------------------
> 
> (Updated 2009-08-06 22:01:42)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> CLI filtering for the gsoc 2009
> 
> This is the first patch to add functionality to filter CLI.
> 
> 
> See usages and examples in doc/cli_filters.txt
> 
> 
> Diffs
> -----
> 
>   /trunk/channels/chan_agent.c 210860 
>   /trunk/channels/chan_alsa.c 210860 
>   /trunk/channels/chan_bridge.c 210860 
>   /trunk/channels/chan_dahdi.c 210860 
>   /trunk/channels/chan_gtalk.c 210860 
>   /trunk/channels/chan_h323.c 210860 
>   /trunk/channels/chan_iax2.c 210860 
>   /trunk/channels/chan_jingle.c 210860 
>   /trunk/channels/chan_local.c 210860 
>   /trunk/channels/chan_mgcp.c 210860 
>   /trunk/channels/chan_misdn.c 210860 
>   /trunk/channels/chan_multicast_rtp.c 210860 
>   /trunk/channels/chan_nbs.c 210860 
>   /trunk/channels/chan_oss.c 210860 
>   /trunk/channels/chan_phone.c 210860 
>   /trunk/channels/chan_skinny.c 210860 
>   /trunk/channels/chan_unistim.c 210860 
>   /trunk/channels/chan_usbradio.c 210860 
>   /trunk/channels/chan_vpb.cc 210860 
>   /trunk/include/asterisk/logger.h 210860 
>   /trunk/include/asterisk/pbx.h 210860 
>   /trunk/include/asterisk/threadstorage.h 210860 
>   /trunk/main/logger.c 210860 
>   /trunk/main/manager.c 210860 
>   /trunk/main/pbx.c 210860 
>   /trunk/main/threadstorage.c 210860 
> 
> Diff: https://reviewboard.asterisk.org/r/336/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> junky
> 
>