[asterisk-dev] [Code Review] Add ability for modules to dynamically register/unregister logger levels

[Tilghman Lesher]

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



/trunk/main/logger.c
<http://reviewboard.digium.com/r/244/#comment1942>

    Same race here.



/trunk/main/logger.c
<http://reviewboard.digium.com/r/244/#comment1940>

    Since you aren't locking dynamic_levels_lock here, there's a potential race condition between the first and second clauses of this conditional, with the unregistration method.
    
    You might reference freed memory or even a NULL pointer, depending upon the stage of the unregistration method.



/trunk/main/logger.c
<http://reviewboard.digium.com/r/244/#comment1939>

    This loop appears to be written to avoid registered levels from duplicating each other or standard levels.  However, since it only starts at level 16, it cannot find conflicts with standard levels (since it skips right over them).



/trunk/main/logger.c
<http://reviewboard.digium.com/r/244/#comment1943>

    I'm not even sure what this mutex around the unregistration is really even protecting.


- Tilghman


On 2009-05-05 10:10:02, Kevin Fleming wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.digium.com/r/244/
> -----------------------------------------------------------
> 
> (Updated 2009-05-05 10:10:02)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> This patch adds the ability for modules to dynamically create logger levels for their own use; these are named levels just like the built-in levels, and can be directed to any destination that the logger can send any level to, by including their names in logger.conf.
> 
> 
> Diffs
> -----
> 
>   /trunk/tests/test_logger.c PRE-CREATION 
>   /trunk/include/asterisk/logger.h 192404 
>   /trunk/main/logger.c 192404 
> 
> Diff: http://reviewboard.digium.com/r/244/diff
> 
> 
> Testing
> -------
> 
> Tested using included test_logger module.
> 
> 
> Thanks,
> 
> Kevin
> 
>