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:20Date Closed:2011-06-07 14:07:28
Versions:Frequency of
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)
    if ((cond) && (option_verbose > level))
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.