[asterisk-dev] 0006889: [patch] Convert ast_verbose into macros

[Tilghman Lesher]

On Thursday 06 April 2006 15:47, Kevin P. Fleming wrote:
> Tilghman Lesher wrote:
> > But the other side of that is it would allow a very low verbose
> > level to be sent to the console, while letting a remote console
> > display a much higher verbose setting.  Currently, if the verbose
> > level is not set high on the console, those extra verbose messages
> > cannot be seen on a remote console.  Additionally, the way that we
> > filter verbose messages to the logger is, at best, a hack:  we scan
> > and trim the leading characters of verbose messages.
>
> While I certainly agree with this idea (and have wanted it for a long
> time), it is definitely not going to happen during this development
> cycle. Given that, whatever we change now can be easily changed
> further in the future; changing from 'if (...) ast_verbose' to a
> single macro actually makes that easier rather than harder.

Unfortunately, his patch doesn't convert it into a single macro; it
converts into 4 different macros, each slightly different, for different
purposes.

> > I honestly don't think the extra messages generate enough load to
> > be concerned on any box that Asterisk is capable of running on,
> > anyway. Use of a single good codec such as iLBC or G.729 is going
> > to generate more load than all the verbose messages in that same
> > timeframe.
>
> Possibly... but there is still value in avoiding the argument
> processing when it can be. For example, many verbose messages call
> translation functions to turn an integer value into a 'pretty' string
> value, but this is entirely avoided if option_verbose is low enough
> to not print the message at all.

That's true, and it's not something this patch takes into consideration.
He'd need a fifth macro for any extra operations which are not
encapsulated in the actual ast_verbose() call, but are instead done
separately inside the if() statement.  For example, there's a few cases
like this in app.c:

    if ((option_verbose > 2) && !warned) {
        ast_verbose(VERBOSE_PREFIX_3 "Message check requested for 
mailbox %s/folder %s but voicemail not loaded.\n", mailbox, folder ? 
folder : "INBOX");
        warned++;
    }

Or another case that cannot be handled by this patch in apps/app_db.c:

    if (option_verbose > 2) {
        if (keytree)
            ast_verbose(VERBOSE_PREFIX_3 "DBdeltree: family=%s, 
keytree=%s\n", family, keytree);
        else
            ast_verbose(VERBOSE_PREFIX_3 "DBdeltree: family=%s\n", 
family);
    }

I agree that macros do, in general, make code easier to read and less
buggy, but in this particular case, these macros only serve to obfuscate
the code.

-- 
Tilghman