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

[Tilghman Lesher]

> On Nov. 29, 2011, 1:39 a.m., wdoekes wrote:
> > /trunk/main/asterisk.c, lines 1753-1756
> > <https://reviewboard.asterisk.org/r/1599/diff/6/?file=21936#file21936line1753>
> >
> >     This check in fix_header should be removed altogether now, right?
> >     
> >     You already incremented it in console_verboser().

Good point.  Removed.


> On Nov. 29, 2011, 1:39 a.m., wdoekes wrote:
> > /trunk/main/asterisk.c, line 1781
> > <https://reviewboard.asterisk.org/r/1599/diff/6/?file=21936#file21936line1781>
> >
> >     If you did intend to have all VERBOSE_PREFIX_n's to have level 0, you can optimize these fix_headers away by checking for:
> >     
> >     if ((level == 0) && (the..rest)) {
> >     
> >     Especially if you expect this not to be used anymore except by third-party apps; but see below.

No, I intend those to be detected all at a common area, within a function.  A lot of these changes are constrained by trying to keep the API consistent with previous versions.  That is, the common macros should remain API-consistent, even if the underlying functions that are actually called are modified to specify additional arguments.  This keeps the changeset low.  Honestly, I'd prefer if things eventually stopped using ast_verbose() altogether in preference for ast_verb().


> On Nov. 29, 2011, 1:39 a.m., wdoekes wrote:
> > /trunk/main/logger.c, lines 1514-1519
> > <https://reviewboard.asterisk.org/r/1599/diff/6/?file=21940#file21940line1514>
> >
> >     > 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.
> >     
> >     Yes. I understand why we scan the string.
> >     
> >     But I don't understand why messages with ast_verb(2) (third log level) get a magic number of -3 (level 2), but those with ast_verbose(VERBOSE_PREFIX_3) get one of -1 (level 0). You might have a good reason for that, but I can't tell what it is from the code.

You're confusing passed parameters with magic values.  Passed level values are in the range 0-127 (or really, higher, but we constrain higher values down to 127) and the special value -1, which indicates the level is to be guessed by scanning the string.

I think you've actually found a flaw in my logic, which is that my compatibility should be detecting levels below 0, instead of just == -1.  I will fix that.


- Tilghman


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


On Nov. 28, 2011, 10:15 a.m., Tilghman Lesher wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1599/
> -----------------------------------------------------------
> 
> (Updated Nov. 28, 2011, 10:15 a.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/20111129/24bfde35/attachment-0001.htm>