[Asterisk-code-review] Replace direct checks of option debug with DEBUG ATLEAST macro. (asterisk[13])

[Corey Farrell]

Corey Farrell has posted comments on this change. ( https://gerrit.asterisk.org/8451 )

Change subject: Replace direct checks of option_debug with DEBUG_ATLEAST macro.
......................................................................


Patch Set 2:

(3 comments)

https://gerrit.asterisk.org/#/c/8451/2/channels/chan_sip.c
File channels/chan_sip.c:

https://gerrit.asterisk.org/#/c/8451/2/channels/chan_sip.c@22343
PS2, Line 22343: 	ast_log(LOG_DEBUG, "\n---------- SIP HISTORY for '%s' \n", dialog->callid);
               : 	if (dialog->subscribed) {
               : 		ast_log(LOG_DEBUG, "  * Subscription\n");
               : 	} else {
               : 		ast_log(LOG_DEBUG, "  * SIP Call\n");
               : 	}
               : 	if (dialog->history) {
               : 		AST_LIST_TRAVERSE(dialog->history, hist, list)
               : 			ast_log(LOG_DEBUG, "  %-3.3d. %s\n", ++x, hist->event);
               : 	}
               : 	if (!x) {
               : 		ast_log(LOG_DEBUG, "Call '%s' has no history\n", dialog->callid);
               : 	}
               : 	ast_log(LOG_DEBUG, "\n---------- END SIP HISTORY for '%s' \n", dialog->callid);
> Heh.  This change appears to fix a bug in the bargain.
So this is just an observation / no action required?


https://gerrit.asterisk.org/#/c/8451/2/main/config.c
File main/config.c:

https://gerrit.asterisk.org/#/c/8451/2/main/config.c@2790
PS2, Line 2790: 		if (!DEBUG_ATLEAST(1)) {
              : 			ast_verb(2, "Saving '%s': saved\n", fn);
              : 		}
> The original test was bizarre.  Only attempt the verbose log if debugging n
I agree it's weird, my intent was for this patch to maintain existing behavior.  I can change it if you want but please be aware this patch does not cherry-pick between 13 and other branches so I'd like for my next revision to be the last.  This is not the only thing that has a pattern like this (ast_verb if not debug).


https://gerrit.asterisk.org/#/c/8451/2/main/sched.c
File main/sched.c:

https://gerrit.asterisk.org/#/c/8451/2/main/sched.c@721
PS2, Line 721: 	ast_log(LOG_DEBUG, "Asterisk Schedule Dump (%zu in Q, %u Total, %u Cache, %u high-water)\n", ast_heap_size(con->sched_heap), con->eventcnt - 1, con->schedccnt, con->highwater);
             : #else
             : 	ast_log(LOG_DEBUG, "Asterisk Schedule Dump (%zu in Q, %u Total, %u high-water)\n", ast_heap_size(con->sched_heap), con->eventcnt - 1, con->highwater);
> I'd be nice if these long lines were broken after the format string. :)
OK will do in next update.



-- 
To view, visit https://gerrit.asterisk.org/8451
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic342d4799a945dbc40ac085ac142681094a4ebf0
Gerrit-Change-Number: 8451
Gerrit-PatchSet: 2
Gerrit-Owner: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-Comment-Date: Wed, 07 Mar 2018 21:45:57 +0000
Gerrit-HasComments: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180307/743042ff/attachment.html>