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

[Corey Farrell]

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

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


Patch Set 1:

(1 comment)

https://gerrit.asterisk.org/#/c/7985/1/main/loader.c
File main/loader.c:

https://gerrit.asterisk.org/#/c/7985/1/main/loader.c@1433
PS1, Line 1433: 	for (v = ast_variable_browse(cfg, "modules"); v; v = v->next) {
              : 		if (find_resource(v->value, 0)) {
              : 			continue;
              : 		}
              : 
              : 		if (!strcasecmp(v->name, preload_only ? "preload" : "load")) {
              : 			add_to_load_order(v->value, &load_order, 0);
              : 		}
              : 
              : 		if (!strcasecmp(v->name, preload_only ? "preload-require" : "require")) {
              : 			/* Add the module to the list and make sure it's required */
              : 			add_to_load_order(v->value, &load_order, 1);
              : 			ast_debug(2, "Adding module to required list: %s (%s)\n", v->value, v->name);
              : 		}
              : 	}
> Remember find_resource() searches a list so it isn't exactly cheap.
I've had second thoughts about this, I'm going to remove this change.  Printing a message if someone configures the system to preload and load a module is not a big deal.  This change could be breaking.  A module could be configured to preload and load, but a 'soft' dependency is not loaded yet (so maybe config load fails and the module declines).  I'm worry this change would cause such a module to be loaded by preload (but not started).  Right now we would try again during regular 'load' stage and possibly succeed, this change would cause the module to remain in a declined state.



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

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-MessageType: comment
Gerrit-Change-Id: I22261599c46d0f416e568910ec9502f45143197f
Gerrit-Change-Number: 7985
Gerrit-PatchSet: 1
Gerrit-Owner: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-Comment-Date: Wed, 17 Jan 2018 16:58:06 +0000
Gerrit-HasComments: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180117/21ef6841/attachment.html>