[asterisk-dev] [Code Review] Replace nested functions with file scope functions

Kevin Fleming kpfleming at digium.com
Thu Nov 6 11:00:23 CST 2008


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/33/#review80
-----------------------------------------------------------



/trunk/apps/app_dial.c
<http://reviewboard.digium.com/r/33/#comment139>

    extremely minor nit... no space between function name and parentheses please.



/trunk/apps/app_directed_pickup.c
<http://reviewboard.digium.com/r/33/#comment140>

    these are duplicate assignments, you assigned the values already in the variable definition.



/trunk/channels/chan_sip.c
<http://reviewboard.digium.com/r/33/#comment145>

    trailing whitespace!



/trunk/channels/chan_sip.c
<http://reviewboard.digium.com/r/33/#comment152>

    This is wrong; when OBJ_POINTER is passed to ao2_callback, it knows that the pointer being passed is to an object that it can hash; 'struct find_by_name_criteria' is not a 'struct sip_peer'. Fixing this will require removing OBJ_POINTER, but then the callback won't be able to hash and look in a single hash bucket, it will have to search the entire table.
    
    Unfortunately ao2_callback doesn't have a facility for both specifying an object to hash on *and* data for the comparison function to use, except by using additional flag bits in the 'flags' argument which is then passed on to the callback function. For now, you'll have to use that method.


- Kevin


On 2008-11-05 19:53:14, Sean Bright wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.digium.com/r/33/
> -----------------------------------------------------------
> 
> (Updated 2008-11-05 19:53:14)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> This is a patch to promote the current nested functions in asterisk trunk to file level functions based on security concerns.  For more details, see:
> 
>   http://gcc.gnu.org/ml/gcc-help/2005-12/msg00174.html
> 
> Also, I added a note to one of the calls in main/features.c which I marked with an XXX comment.
> 
> 
> Diffs
> -----
> 
>   /trunk/apps/app_dial.c 154920 
>   /trunk/apps/app_directed_pickup.c 154926 
>   /trunk/apps/app_followme.c 154920 
>   /trunk/apps/app_queue.c 154920 
>   /trunk/channels/chan_sip.c 154920 
>   /trunk/include/asterisk/channel.h 154920 
>   /trunk/main/channel.c 154920 
>   /trunk/main/features.c 154923 
> 
> Diff: http://reviewboard.digium.com/r/33/diff
> 
> 
> Testing
> -------
> 
> This passes the 'make' test with dev-mode enabled, but I have not functionally tested extensively.
> 
> 
> Thanks,
> 
> Sean
> 
>




More information about the asterisk-dev mailing list