[Asterisk-code-review] loader: Refactor resource name match. (asterisk[master])

Jenkins2 asteriskteam at digium.com
Mon Dec 11 11:44:35 CST 2017


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

Change subject: loader: Refactor resource_name_match.
......................................................................

loader: Refactor resource_name_match.

Optimize resource_name_match.  This change eliminates use of
ast_strdupa, instead verifying that both basename's are the same length,
then using strncasecmp.

Change-Id: I477275c0e954c99d74be5abfc8bb6545b04e5a3d
---
M main/loader.c
1 file changed, 29 insertions(+), 18 deletions(-)

Approvals:
  Joshua Colp: Looks good to me, but someone else must approve
  Sean Bright: 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 2238975..49151cc 100644
--- a/main/loader.c
+++ b/main/loader.c
@@ -376,35 +376,39 @@
 	return -1;
 }
 
-static int resource_name_match(const char *name1_in, const char *name2_in)
+static size_t resource_name_baselen(const char *name)
 {
-	char *name1 = (char *) name1_in;
-	char *name2 = (char *) name2_in;
+	size_t len = strlen(name);
 
-	/* trim off any .so extensions */
-	if (!strcasecmp(name1 + strlen(name1) - 3, ".so")) {
-		name1 = ast_strdupa(name1);
-		name1[strlen(name1) - 3] = '\0';
-	}
-	if (!strcasecmp(name2 + strlen(name2) - 3, ".so")) {
-		name2 = ast_strdupa(name2);
-		name2[strlen(name2) - 3] = '\0';
+	if (len > 3 && !strcasecmp(name + len - 3, ".so")) {
+		return len - 3;
 	}
 
-	return strcasecmp(name1, name2);
+	return len;
+}
+
+static int resource_name_match(const char *name1, size_t baselen1, const char *name2)
+{
+	if (baselen1 != resource_name_baselen(name2)) {
+		return -1;
+	}
+
+	return strncasecmp(name1, name2, baselen1);
 }
 
 static struct ast_module *find_resource(const char *resource, int do_lock)
 {
 	struct ast_module *cur;
+	size_t resource_baselen = resource_name_baselen(resource);
 
 	if (do_lock) {
 		AST_DLLIST_LOCK(&module_list);
 	}
 
 	AST_DLLIST_TRAVERSE(&module_list, cur, entry) {
-		if (!resource_name_match(resource, cur->resource))
+		if (!resource_name_match(resource, resource_baselen, cur->resource)) {
 			break;
+		}
 	}
 
 	if (do_lock) {
@@ -938,6 +942,7 @@
 	struct ast_module *cur;
 	enum ast_module_reload_result res = AST_MODULE_RELOAD_NOT_FOUND;
 	int i;
+	size_t name_baselen = name ? resource_name_baselen(name) : 0;
 
 	/* If we aren't fully booted, we just pretend we reloaded but we queue this
 	   up to run once we are booted up. */
@@ -991,8 +996,9 @@
 	AST_DLLIST_TRAVERSE(&module_list, cur, entry) {
 		const struct ast_module_info *info = cur->info;
 
-		if (name && resource_name_match(name, cur->resource))
+		if (name && resource_name_match(name, name_baselen, cur->resource)) {
 			continue;
+		}
 
 		if (!cur->flags.running || cur->flags.declined) {
 			if (res == AST_MODULE_RELOAD_NOT_FOUND) {
@@ -1186,9 +1192,10 @@
 static struct load_order_entry *add_to_load_order(const char *resource, struct load_order *load_order, int required)
 {
 	struct load_order_entry *order;
+	size_t resource_baselen = resource_name_baselen(resource);
 
 	AST_LIST_TRAVERSE(load_order, order, entry) {
-		if (!resource_name_match(order->resource, resource)) {
+		if (!resource_name_match(resource, resource_baselen, order->resource)) {
 			/* Make sure we have the proper setting for the required field
 			   (we might have both load= and required= lines in modules.conf) */
 			order->required |= required;
@@ -1435,11 +1442,15 @@
 	/* now scan the config for any modules we are prohibited from loading and
 	   remove them from the load order */
 	for (v = ast_variable_browse(cfg, "modules"); v; v = v->next) {
-		if (strcasecmp(v->name, "noload"))
-			continue;
+		size_t baselen;
 
+		if (strcasecmp(v->name, "noload")) {
+			continue;
+		}
+
+		baselen = resource_name_baselen(v->value);
 		AST_LIST_TRAVERSE_SAFE_BEGIN(&load_order, order, entry) {
-			if (!resource_name_match(order->resource, v->value)) {
+			if (!resource_name_match(v->value, baselen, order->resource)) {
 				AST_LIST_REMOVE_CURRENT(entry);
 				ast_free(order->resource);
 				ast_free(order);

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

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I477275c0e954c99d74be5abfc8bb6545b04e5a3d
Gerrit-Change-Number: 7501
Gerrit-PatchSet: 3
Gerrit-Owner: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Sean Bright <sean.bright at gmail.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20171211/d3b28302/attachment.html>


More information about the asterisk-code-review mailing list