[Asterisk-code-review] PJPROJECT logging: Fix detection of max supported log level. (asterisk[13])

Richard Mudgett asteriskteam at digium.com
Mon Jan 23 12:01:16 CST 2017


Richard Mudgett has uploaded a new change for review. ( https://gerrit.asterisk.org/4767 )

Change subject: PJPROJECT logging: Fix detection of max supported log level.
......................................................................

PJPROJECT logging: Fix detection of max supported log level.

The mechanism used for detecting the maximum log level compiled into the
linked pjproject did not work.  The API call simply stores the requested
level into an integer and does no range checking.  Asterisk was assuming
that there was range checking and limited the new value to the allowable
range.  To get the actual maximum log level compiled into the linked
pjproject we need to get and save off the initial set log level from
pjproject.  This is the maximum log level supported.

* Get and save off the initial log level setting before altering it to the
desired level on startup.  This has to be done by a macro rather than
calling a core function to avoid incorrectly linking pjproject.

* Split the initial log level warning messages to warn if the linked
pjproject cannot support the requested startup level and if it is too low
to get the pjproject buildopts for "pjproject show buildopts".

* Adjust the CLI "pjproject set log level" to check the saved max log
level and to generate normal output messages instead of a warning message.

ASTERISK-26743 #close

Change-Id: I40aa76653e2a1dece66c3f8734594b4f0471cfb4
---
M include/asterisk/options.h
M main/asterisk.c
M main/libasteriskpj.c
M res/res_pjproject.c
M res/res_rtp_asterisk.c
5 files changed, 39 insertions(+), 11 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/67/4767/1

diff --git a/include/asterisk/options.h b/include/asterisk/options.h
index 0f8c6f8..748833e 100644
--- a/include/asterisk/options.h
+++ b/include/asterisk/options.h
@@ -143,6 +143,22 @@
  */
 #define DEFAULT_PJ_LOG_MAX_LEVEL	2
 
+/*!
+ * Determine the maximum log level the pjproject we linked with supports.
+ *
+ * This must be done before we ever try to change the level.
+ */
+#define AST_PJPROJECT_INIT_LOG_LEVEL()							\
+	do {														\
+		if (!ast_pjproject_max_log_level) {						\
+			ast_pjproject_max_log_level = pj_log_get_level();	\
+		}														\
+		pj_log_set_level(ast_option_pjproject_log_level);		\
+	} while (0)
+
+/*! Current linked pjproject maximum logging level */
+extern int ast_pjproject_max_log_level;
+
 /*! Current pjproject logging level */
 extern int ast_option_pjproject_log_level;
 
diff --git a/main/asterisk.c b/main/asterisk.c
index 2236e8c..2b1c2fd 100644
--- a/main/asterisk.c
+++ b/main/asterisk.c
@@ -325,6 +325,7 @@
 
 int option_verbose;				/*!< Verbosity level */
 int option_debug;				/*!< Debug level */
+int ast_pjproject_max_log_level;
 int ast_option_pjproject_log_level;
 double ast_option_maxload;			/*!< Max load avg on system */
 int ast_option_maxcalls;			/*!< Max number of active calls */
diff --git a/main/libasteriskpj.c b/main/libasteriskpj.c
index 2d92b59..80c4d18 100644
--- a/main/libasteriskpj.c
+++ b/main/libasteriskpj.c
@@ -47,7 +47,7 @@
 int ast_pj_init(void)
 {
 #ifdef HAVE_PJPROJECT_BUNDLED
-	pj_log_set_level(ast_option_pjproject_log_level);
+	AST_PJPROJECT_INIT_LOG_LEVEL();
 	pj_init();
 #endif
 	return 0;
diff --git a/res/res_pjproject.c b/res/res_pjproject.c
index cd56266..e02515b 100644
--- a/res/res_pjproject.c
+++ b/res/res_pjproject.c
@@ -412,17 +412,21 @@
 	}
 
 	/* Update pjproject logging level */
+	if (ast_pjproject_max_log_level < level_new) {
+		level_new = ast_pjproject_max_log_level;
+		ast_cli(a->fd,
+			"Asterisk built or linked with pjproject PJ_LOG_MAX_LEVEL=%d.\n"
+			"Lowering request to the max supported level.\n",
+			ast_pjproject_max_log_level);
+	}
 	level_old = ast_option_pjproject_log_level;
 	if (level_old == level_new) {
 		ast_cli(a->fd, "pjproject log level is still %d.\n", level_old);
 	} else {
 		ast_cli(a->fd, "pjproject log level was %d and is now %d.\n",
 			level_old, level_new);
+		ast_option_pjproject_log_level = level_new;
 		pj_log_set_level(level_new);
-	}
-	ast_option_pjproject_log_level = pj_log_get_level();
-	if (ast_option_pjproject_log_level != level_new) {
-		ast_log(LOG_WARNING, "Asterisk built with pjproject PJ_LOG_MAX_LEVEL set too low.\n");
 	}
 
 	return CLI_SUCCESS;
@@ -497,7 +501,7 @@
 
 	ast_sorcery_load(pjproject_sorcery);
 
-	pj_log_set_level(ast_option_pjproject_log_level);
+	AST_PJPROJECT_INIT_LOG_LEVEL();
 	pj_init();
 
 	decor_orig = pj_log_get_decor();
@@ -514,12 +518,19 @@
 	pj_log_set_decor(0);
 	pj_log_set_level(MAX_PJ_LOG_MAX_LEVEL);/* Set level to guarantee the dump output. */
 	pj_dump_config();
-	pj_log_set_level(ast_option_pjproject_log_level);
 	pj_log_set_decor(PJ_LOG_HAS_SENDER | PJ_LOG_HAS_INDENT);
 	pj_log_set_log_func(log_forwarder);
-	if (!AST_VECTOR_SIZE(&buildopts)
-		|| ast_option_pjproject_log_level != pj_log_get_level()) {
-		ast_log(LOG_WARNING, "Asterisk built or linked with pjproject PJ_LOG_MAX_LEVEL set too low.\n");
+	if (ast_pjproject_max_log_level < ast_option_pjproject_log_level) {
+		ast_log(LOG_WARNING,
+			"Asterisk built or linked with pjproject PJ_LOG_MAX_LEVEL=%d which is too low for startup level: %d.\n",
+			ast_pjproject_max_log_level, ast_option_pjproject_log_level);
+		ast_option_pjproject_log_level = ast_pjproject_max_log_level;
+	}
+	pj_log_set_level(ast_option_pjproject_log_level);
+	if (!AST_VECTOR_SIZE(&buildopts)) {
+		ast_log(LOG_NOTICE,
+			"Asterisk built or linked with pjproject PJ_LOG_MAX_LEVEL=%d which is too low to get buildopts.\n",
+			ast_pjproject_max_log_level);
 	}
 
 	ast_cli_register_multiple(pjproject_cli, ARRAY_LEN(pjproject_cli));
diff --git a/res/res_rtp_asterisk.c b/res/res_rtp_asterisk.c
index 2409e3a..01b60b2 100644
--- a/res/res_rtp_asterisk.c
+++ b/res/res_rtp_asterisk.c
@@ -5669,7 +5669,7 @@
 #ifdef HAVE_PJPROJECT
 	pj_lock_t *lock;
 
-	pj_log_set_level(ast_option_pjproject_log_level);
+	AST_PJPROJECT_INIT_LOG_LEVEL();
 	if (pj_init() != PJ_SUCCESS) {
 		return AST_MODULE_LOAD_DECLINE;
 	}

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I40aa76653e2a1dece66c3f8734594b4f0471cfb4
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>



More information about the asterisk-code-review mailing list