Posted: Mon May 04, 2009 11:58 am Post subject: [asterisk-dev] [Code Review] Add ability for modules to dyna
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/244/
-----------------------------------------------------------
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.
Posted: Tue May 05, 2009 1:31 pm Post subject: [asterisk-dev] [Code Review] Add ability for modules to dyna
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/244/
-----------------------------------------------------------
(Updated 2009-05-05 09:20:44.556149)
Review request for Asterisk Developers.
Changes
-------
New version of patch that eliminates the need for flushing messages queued for the background thread by copying the additional data necessary into the structure that is passed to that thread.
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.
Posted: Tue May 05, 2009 1:57 pm Post subject: [asterisk-dev] [Code Review] Add ability for modules to dyna
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/244/#review766
-----------------------------------------------------------
This is a bit inefficient since the loop will continue past the point that you find the first available dynamic level. You should break instead of continuing.
I think you should make a copy of the name here since the parameter passed to this function may go out of scope while the log level is still registered. Of course, this would mean that a corresponding free should be added to the unregister function.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/244/
-----------------------------------------------------------
(Updated 2009-05-05 09:20:44)
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.
Posted: Tue May 05, 2009 2:19 pm Post subject: [asterisk-dev] [Code Review] Add ability for modules to dyna
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/244/
-----------------------------------------------------------
(Updated 2009-05-05 10:10:02.372067)
Review request for Asterisk Developers.
Changes
-------
Addressed Mark's comments.
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.
Posted: Tue May 05, 2009 2:31 pm Post subject: [asterisk-dev] [Code Review] Add ability for modules to dyna
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/244/#review769
-----------------------------------------------------------
Ship it!
- Mark
On 2009-05-05 10:10:02, Kevin Fleming wrote:
Quote:
-----------------------------------------------------------
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.
Posted: Wed May 06, 2009 7:58 pm Post subject: [asterisk-dev] [Code Review] Add ability for modules to dyna
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/244/#review771
-----------------------------------------------------------
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.
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).
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:
Quote:
-----------------------------------------------------------
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.
Posted: Thu May 14, 2009 9:40 pm Post subject: [asterisk-dev] [Code Review] Add ability for modules to dyna
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/244/
-----------------------------------------------------------
(Updated 2009-05-14 17:30:06.135530)
Review request for Asterisk Developers.
Changes
-------
Addressed Tilghman's comments.
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.
Posted: Thu May 14, 2009 9:40 pm Post subject: [asterisk-dev] [Code Review] Add ability for modules to dyna
Quote:
On 2009-05-05 09:46:44, Mark Michelson wrote:
> /trunk/main/logger.c, lines 1369-1373
> <http://reviewboard.digium.com/r/244/diff/2/?file=5018#file5018line1369>
>
> This is a bit inefficient since the loop will continue past the point that you find the first available dynamic level. You should break instead of continuing.
Actually, that's intentional; the second part of the loop needs to check every non-blank entry in the array for possible duplicates of the level being registered.
Quote:
On 2009-05-05 09:46:44, Mark Michelson wrote:
> /trunk/main/logger.c, line 1394
> <http://reviewboard.digium.com/r/244/diff/2/?file=5018#file5018line1394>
>
> I think you should make a copy of the name here since the parameter passed to this function may go out of scope while the log level is still registered. Of course, this would mean that a corresponding free should be added to the unregister function.
Will fix.
Quote:
On 2009-05-05 09:46:44, Mark Michelson wrote:
> /trunk/main/logger.c, lines 119-129
> <http://reviewboard.digium.com/r/244/diff/2/?file=5018#file5018line119>
>
> These comments should be doxygenified.
Done.
- Kevin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/244/#review766
-----------------------------------------------------------
On 2009-05-14 17:30:06, Kevin Fleming wrote:
Quote:
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/244/
-----------------------------------------------------------
(Updated 2009-05-14 17:30:06)
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.
Posted: Thu May 14, 2009 11:41 pm Post subject: [asterisk-dev] [Code Review] Add ability for modules to dyna
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/244/#review781
-----------------------------------------------------------
Ship it!
Looks good, here.
- Tilghman
On 2009-05-14 17:30:06, Kevin Fleming wrote:
Quote:
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/244/
-----------------------------------------------------------
(Updated 2009-05-14 17:30:06)
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.
You cannot post new topics in this forum You cannot reply to topics in this forum You cannot edit your posts in this forum You cannot delete your posts in this forum You cannot vote in polls in this forum