[asterisk-dev] [Code Review]: Separate verbose level for logging

[Tilghman Lesher]

> On Nov. 28, 2011, 2:15 a.m., wdoekes wrote:
> > /trunk/include/asterisk/logger.h, line 74
> > <https://reviewboard.asterisk.org/r/1599/diff/5/?file=21911#file21911line74>
> >
> >     s/abscence/absence/
> >     
> >     Not your typo, but you're in the neighbourhood.

Fixed.


> On Nov. 28, 2011, 2:15 a.m., wdoekes wrote:
> > /trunk/main/asterisk.c, lines 1748-1753
> > <https://reviewboard.asterisk.org/r/1599/diff/5/?file=21912#file21912line1748>
> >
> >     You don't want to do this without casting s to a signed char.
> >     
> >     On some platforms char is unsigned by default.

Fixed.  Interesting legacy discussion, which means there are 3 distinct char types in C.  Did not realize this.  Unfortunately, I can't use -pedantic to test the change, because we do a lot of other things for which -pedantic fails.


> On Nov. 28, 2011, 2:15 a.m., wdoekes wrote:
> > /trunk/main/logger.c, lines 1514-1519
> > <https://reviewboard.asterisk.org/r/1599/diff/5/?file=21916#file21916line1514>
> >
> >     Signed char here, or cast. (It won't matter, but might generate a warning.)
> >     
> >     And perhaps a short explanation above this about when level and magic is which.
> >     
> >     I can't tell here who (and why someone) would pass a negative or very high level here, or is that simply bounds checking?
> >     
> >     From the code above, I gather that "-1" is passed when we're using the old-style VERBOSE_PREFIX_*.
> >     
> >     Shouldn't the magic number be calculated after the re-setting of level that happens in the level == -1 if.

Fixed.

It's just bounds checking.  If we're passed a negative level (see the ast_verbose() macro), then we want to scan the string to detect verbose level.  So yes, the ordering was intentional.


- Tilghman


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1599/#review4871
-----------------------------------------------------------


On Nov. 27, 2011, 8:33 p.m., Tilghman Lesher wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1599/
> -----------------------------------------------------------
> 
> (Updated Nov. 27, 2011, 8:33 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> One problem that many admins have today is that to have maximum verbosity going to the logs, for later debugging of trouble calls, the console becomes virtually unusable, because the verbosity level is global.  This patch attempts to solve that problem, allowing each verbose recipient to have its own filter as to what it wants to receive.
> 
> 
> Diffs
> -----
> 
>   /trunk/addons/chan_ooh323.c 346290 
>   /trunk/addons/res_config_mysql.c 346290 
>   /trunk/apps/app_rpt.c 346290 
>   /trunk/apps/app_verbose.c 346290 
>   /trunk/apps/app_voicemail.c 346290 
>   /trunk/channels/chan_sip.c 346290 
>   /trunk/channels/chan_skinny.c 346290 
>   /trunk/channels/chan_usbradio.c 346290 
>   /trunk/codecs/codec_dahdi.c 346290 
>   /trunk/configs/logger.conf.sample 346290 
>   /trunk/include/asterisk/logger.h 346290 
>   /trunk/main/asterisk.c 346290 
>   /trunk/main/bridging.c 346290 
>   /trunk/main/cli.c 346290 
>   /trunk/main/dial.c 346290 
>   /trunk/main/logger.c 346290 
>   /trunk/main/say.c 346290 
>   /trunk/res/res_clialiases.c 346290 
>   /trunk/res/res_fax.c 346290 
>   /trunk/res/res_jabber.c 346290 
>   /trunk/res/res_musiconhold.c 346290 
> 
> Diff: https://reviewboard.asterisk.org/r/1599/diff
> 
> 
> Testing
> -------
> 
> Verified logging with files, consoles with different verbosity levels, etc.
> 
> 
> Thanks,
> 
> Tilghman
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20111128/04833663/attachment-0001.htm>