[asterisk-dev] [Code Review] Convert open coded lists in indications to linked list macros

[Russell Bryant]

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



/trunk/include/asterisk/indications.h
<http://reviewboard.digium.com/r/34/#comment155>

    Unless you need to declare another list head for ind_tone_zone_sound structs, you can make this definition typeless.  Basically, ...
    
    AST_LIST_HEAD_NOLOCK(, ind_tone_zone_sound) tones;



/trunk/main/indications.c
<http://reviewboard.digium.com/r/34/#comment156>

    The old code inserted the element at the head of the list, not the tail



/trunk/main/indications.c
<http://reviewboard.digium.com/r/34/#comment157>

    The code already makes provisions such that an indication will only be in the list once.  You can break after finding and destroying it.



/trunk/res/res_indications.c
<http://reviewboard.digium.com/r/34/#comment158>

    Your code did not affect this, but reviewing this patch made me realize that object management and protection in this indications stuff is totally broken.
    
    There is nothing to ensure that the result from ast_walk_indications() is still valid at any point in time.
    
    There is also no locking being done on an indication to ensure that the contents do not change while you're accessing them ...
    
    This is really an issue of the manage of the indications, not the tones inside of them, which is what you're converting.  So for now, we can proceed with your list conversion for the tones.  However, we need to rework the indications object management, later.



/trunk/res/res_indications.c
<http://reviewboard.digium.com/r/34/#comment159>

    Again, you changed a head insert to a tail insert


- Russell


On 2008-11-06 10:32:45, Sean Bright wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.digium.com/r/34/
> -----------------------------------------------------------
> 
> (Updated 2008-11-06 10:32:45)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> 
> Diffs
> -----
> 
>   /trunk/include/asterisk/indications.h 154965 
>   /trunk/main/indications.c 154965 
>   /trunk/res/res_indications.c 154965 
> 
> Diff: http://reviewboard.digium.com/r/34/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sean
> 
>