Summary:	ASTERISK-06706: [patch][post-1.4] Convert ast_verbose into macros?
Reporter:	Andrey S Pankov (casper)	Labels:
Date Opened:	2006-04-05 10:39:20	Date Closed:	2011-06-07 14:07:28
Priority:	Minor	Regression?	No
Status:	Closed/Complete	Components:	Core/General
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) 21180.patch ( 1) logger.h.2.diff ( 2) logger.h.diff
Description:	I'd like to hear back form the developer community if converting blocks like:     if (option_verbose > level)          ast_verbose("\n"); or:     if ((cond) && (option_verbose > level))          ast_verbose("\n"); into macros:     AST_VERBOSE(level, "");     AST_VERBOSE_IF(level, cond, ""); would be a handy addition to avoid errors such as accidental inclusion of code into if controls and not terminating verbose output with "\n".
Comments:	By: Olle Johansson (oej) 2006-04-05 10:57:58 Send this question to the asterisk-dev mailing list. The bug tracker is not a good place for discussion... Thanks. I think the idea looks promising. By: Andrey S Pankov (casper) 2006-04-05 11:42:31 Please change the summary back to be "[patch] Convert ast_verbose into macros" since I uploaded the patch... By: Andrey S Pankov (casper) 2006-04-05 11:51:43 I'm not on asterisk-dev, nor on irc. I'm sure this is not worse "discussion" at all. I'll do the conversion for entire asterisk tree myself... By: Olle Johansson (oej) 2006-04-05 11:53:16 It's time you joined the asterisk-dev mailing list. By: Andrey S Pankov (casper) 2006-04-05 13:05:06 Please do not close this bug. I hope "feedback" is the appropriate status for issues waiting for feedback or approval... By: Olle Johansson (oej) 2006-04-05 13:06:14 Casper: Two bug marshals already closed this, instructing you to take the discussion to other tools we have provided for the community. The bug tracker is not a discussion forum, it's a tool for us. By: Andrey S Pankov (casper) 2006-04-09 20:57:46 I re-open it since kpfleming is "not opposed to the macros as proposed". I upload the macros fixed as he suggested. By: Andrey S Pankov (casper) 2006-04-09 21:13:19 AST_VERBOSE_main.diff is for trunk r18699 main dir only. Sorry, logger.c changes include some formatting fixes. By: Olle Johansson (oej) 2006-04-12 10:15:24 casper, I've told you on the mailing list to be patient. This is not a critical change at all. By: Andrey S Pankov (casper) 2006-04-12 12:58:28 Why mark this as post-1.4? It's ready to be commited (I mean the macros only) and it was ready before this Friday. Such a convertion will facilitate further changes in any case as Kevin noted. I promise to do conversions by the end of the week if needed (they are almost ready for entire source tree). I do not plan to convert any channel drivers since their debug logic is more complex and this will be addressed in 1.6 dev cycle anyway I hope. By: Andrey S Pankov (casper) 2006-04-17 03:54:28 I'll not maintain this any longer. Thanks you, guys, for the patience. Mithraen is the official maintainer for the patch now. By: Denis Smirnov (mithraen) 2006-04-27 05:36:20 Patch updated to last trunk By: Serge Vecher (serge-v) 2006-05-08 09:22:29 Mithraen: please get some testers for this patch and perhaps the fact that the patch is tested and confirmed kosher by 3-rd parties will tip the scales. By: Denis Smirnov (mithraen) 2006-05-08 10:00:11 vechers, please look into this patch. Patches like this don't need _testing_, only architecture and code review. By: Jason Parker (jparker) 2006-09-01 14:07:10 Maybe I'm missing something extremely obvious - but I don't see AST_VERBOSE_IF defined anywhere? By: Denis Smirnov (mithraen) 2006-09-01 15:44:41 Oops. It seems like part of this patch was lost (that needed to be in inculde/) By: Denis Smirnov (mithraen) 2006-09-01 15:46:21 I now have only part, that was in root dir. Part that was in include/ was lost? By: Denis Smirnov (mithraen) 2006-09-03 13:38:37 Patch was lost :( By: Andrey S Pankov (casper) 2007-06-12 11:34:30 Any chance for AST_VERBOSE macros to be in the tree after rev.68987 commit for issue ASTERISK-9628? ;) By: Tilghman Lesher (tilghman) 2007-06-12 13:52:56 The only way that I can see this going is: ast_verbose(level, ...) translated into if (opt_verbose >= level)    ast_verbose(AST_VERBOSE_PREFIX..., __ARGS__) The AST_VERBOSE_IF macro still makes the logic more complicated, not less so. By: Jason Parker (jparker) 2007-06-12 15:50:39 #define ast_verbose(level, ...) do {       \ if (option_verbose >= (level)) {       \  _ast_verbose(VERBOSE_PREFIX_##level, __VA_ARGS__); \  }                                    \ } while (0) Would something like that not work?  It's mostly stolen from ast_debug, but...yeah.  I'm pretty sure it's ## that I'm thinking of... By: Andrey S Pankov (casper) 2007-06-13 05:10:55 qwell: Please note that only four VERBOSE_PREFIX_x are defined and ast_verbose is also used to produce channel debug output (e.g. sip debug)... Corydon76: an advantage of AST_VERBOSE_IF macro is that it first checks verbosity level and then a (possibly complicated) condition. By: Tilghman Lesher (tilghman) 2007-06-13 09:06:14 casper:  the way forward is without the _IF macro. By: Andrey S Pankov (casper) 2007-06-13 10:50:09 Patch uploaded. Disclaimer is on file. By: Jason Parker (jparker) 2007-07-23 15:23:09 ast_verb, which is very similar to this, has been added to svn trunk.