[Asterisk-code-review] loader: Miscellaneous fixes. (asterisk[13])

[Jenkins2]

Jenkins2 has submitted this change and it was merged. ( https://gerrit.asterisk.org/7985 )

Change subject: loader: Miscellaneous fixes.
......................................................................

loader: Miscellaneous fixes.

* Remove comment about lazy load.
* Improve message about module already being loaded and running.
* Handle allocation error in add_to_load_order.
* Dead code elimination from modules_shutdown.

Change-Id: I22261599c46d0f416e568910ec9502f45143197f
---
M main/loader.c
1 file changed, 12 insertions(+), 16 deletions(-)

Approvals:
  Richard Mudgett: Looks good to me, but someone else must approve
  Benjamin Keith Ford: Looks good to me, but someone else must approve
  Kevin Harwell: Looks good to me, approved
  Jenkins2: Approved for Submit



diff --git a/main/loader.c b/main/loader.c
index b9baf3e..6119d63 100644
--- a/main/loader.c
+++ b/main/loader.c
@@ -530,9 +530,6 @@
 	 * If somehow there was another dlopen() on the same module (unlikely,
 	 * since that all is supposed to happen in loader.c).
 	 *
-	 * Or the lazy resolution of a global symbol (very likely, since that is
-	 * how we load all of our modules that export global symbols).
-	 *
 	 * Avoid the temptation of repeating the dlclose(). The other code that
 	 * dlopened the module still has its module reference, and should close
 	 * it itself. In other situations, dlclose() will happily return success
@@ -637,7 +634,8 @@
 int modules_shutdown(void)
 {
 	struct ast_module *mod;
-	int somethingchanged = 1, final = 0;
+	int somethingchanged;
+	int res;
 
 	AST_DLLIST_LOCK(&module_list);
 
@@ -645,17 +643,11 @@
 	 * may be still in use, even if all channels are dead.  We must therefore
 	 * check the usecount before asking modules to unload. */
 	do {
-		if (!somethingchanged) {
-			/*!\note If we go through the entire list without changing
-			 * anything, ignore the usecounts and unload, then exit. */
-			final = 1;
-		}
-
 		/* Reset flag before traversing the list */
 		somethingchanged = 0;
 
 		AST_DLLIST_TRAVERSE_BACKWARDS_SAFE_BEGIN(&module_list, mod, entry) {
-			if (!final && mod->usecount) {
+			if (mod->usecount) {
 				ast_debug(1, "Passing on %s: its use count is %d\n",
 					mod->resource, mod->usecount);
 				continue;
@@ -678,12 +670,12 @@
 				}
 			}
 		}
-	} while (somethingchanged && !final);
+	} while (somethingchanged);
 
-	final = AST_DLLIST_EMPTY(&module_list);
+	res = AST_DLLIST_EMPTY(&module_list);
 	AST_DLLIST_UNLOCK(&module_list);
 
-	return !final;
+	return !res;
 }
 
 int ast_unload_resource(const char *resource_name, enum ast_module_unload_mode force)
@@ -1175,7 +1167,7 @@
 
 	if ((mod = find_resource(resource_name, 0))) {
 		if (mod->flags.running) {
-			ast_log(LOG_WARNING, "Module '%s' already exists.\n", resource_name);
+			ast_log(LOG_WARNING, "Module '%s' already loaded and running.\n", resource_name);
 			return AST_MODULE_LOAD_DECLINE;
 		}
 		if (global_symbols_only && !ast_test_flag(mod->info, AST_MODFLAG_GLOBAL_SYMBOLS))
@@ -1255,6 +1247,11 @@
 		return NULL;
 
 	order->resource = ast_strdup(resource);
+	if (!order->resource) {
+		ast_free(order);
+
+		return NULL;
+	}
 	order->required = required;
 	AST_LIST_INSERT_TAIL(load_order, order, entry);
 
@@ -1437,7 +1434,6 @@
 			add_to_load_order(v->value, &load_order, 1);
 			ast_debug(2, "Adding module to required list: %s (%s)\n", v->value, v->name);
 		}
-
 	}
 
 	/* check if 'autoload' is on */

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

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-MessageType: merged
Gerrit-Change-Id: I22261599c46d0f416e568910ec9502f45143197f
Gerrit-Change-Number: 7985
Gerrit-PatchSet: 2
Gerrit-Owner: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Benjamin Keith Ford <bford at digium.com>
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180118/24e42695/attachment.html>