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

[Russell Bryant]

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



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

    Remove the space after the function name.
    
    Also, it's worth noting that I think this was introduced in 1.4, as well, so we need to fix it there as well once this stuff gets finalized.



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

    The cast is not needed.



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

    Cast is not needed here



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

    No cast



/trunk/apps/app_followme.c
<http://reviewboard.digium.com/r/33/#comment147>

    Remove space after func name



/trunk/apps/app_followme.c
<http://reviewboard.digium.com/r/33/#comment148>

    no cast



/trunk/apps/app_queue.c
<http://reviewboard.digium.com/r/33/#comment149>

    no cast



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

    no cast



/trunk/main/features.c
<http://reviewboard.digium.com/r/33/#comment151>

    no cast



/trunk/main/features.c
<http://reviewboard.digium.com/r/33/#comment153>

    This is a very good question.
    
    ast_pickup_call() will be called by a channel driver for a channel that has made an incoming call, but a PBX thread (channel thread) has _not_ been started on the channel.  So, there is no other thread that can hang up on this channel out from under you and make it become invalid.
    
    The code is perfectly safe how it is.  In theory, the value of pickupgroup could change in the middle of your search, but that doesn't really matter.  There is no need to make special accommodations for that edge case. 


- Russell


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
> 
>