<p>Corey Farrell <strong>posted comments</strong> on this change.</p><p><a href="https://gerrit.asterisk.org/7985">View Change</a></p><p>Patch set 1:</p><p>(1 comment)</p><ul style="list-style: none; padding-left: 20px;"><li><p><a href="https://gerrit.asterisk.org/#/c/7985/1/main/loader.c">File main/loader.c:</a></p><ul style="list-style: none; padding-left: 20px;"><li><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/7985/1/main/loader.c@1433">Patch Set #1, Line 1433:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;"> for (v = ast_variable_browse(cfg, "modules"); v; v = v->next) {<br> if (find_resource(v->value, 0)) {<br> continue;<br> }<br><br> if (!strcasecmp(v->name, preload_only ? "preload" : "load")) {<br> add_to_load_order(v->value, &load_order, 0);<br> }<br><br> if (!strcasecmp(v->name, preload_only ? "preload-require" : "require")) {<br> /* Add the module to the list and make sure it's required */<br> add_to_load_order(v->value, &load_order, 1);<br> ast_debug(2, "Adding module to required list: %s (%s)\n", v->value, v->name);<br> }<br> }<br></pre></blockquote></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Remember find_resource() searches a list so it isn't exactly cheap.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">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.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/7985">change 7985</a>. To unsubscribe, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/7985"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 13 </div>
<div style="display:none"> Gerrit-MessageType: comment </div>
<div style="display:none"> Gerrit-Change-Id: I22261599c46d0f416e568910ec9502f45143197f </div>
<div style="display:none"> Gerrit-Change-Number: 7985 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Corey Farrell <git@cfware.com> </div>
<div style="display:none"> Gerrit-Reviewer: Corey Farrell <git@cfware.com> </div>
<div style="display:none"> Gerrit-Reviewer: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins2 </div>
<div style="display:none"> Gerrit-Reviewer: Richard Mudgett <rmudgett@digium.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Wed, 17 Jan 2018 16:58:06 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>