<p>Richard Mudgett <strong>posted comments</strong> on this change.</p><p><a href="https://gerrit.asterisk.org/7985">View Change</a></p><p>Patch set 1:<span style="border-radius: 3px; display: inline-block; margin: 0 2px; padding: 4px;background-color: #ffd4d4;">Code-Review -1</span></p><p>(2 comments)</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@678">Patch Set #1, Line 678:</a> <code style="font-family:monospace,monospace">    } while (somethingchanged && !final);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">We are currently staying in the loop only while things are changing.  The "final" variable is meaningless because it can never get set.  It has been this way since the original code was added by commit c0b3c923a46afc12837698a78a11b17a7fc384d8.  The "final" variable and code controlled by it can be removed as dead code.</p><p style="white-space: pre-wrap; word-wrap: break-word;">Not forcing the unload of modules with a use count appears to be intensional since the return value is true if the module list is not empty.  The return value is used by the caller to switch to the fast shutdown mode (Commit f373de302032c13487cfcaa616fc070f10d68b57).</p><p style="white-space: pre-wrap; word-wrap: break-word;">Should fix this in all branches.</p><p style="white-space: pre-wrap; word-wrap: break-word;">Remove final and update the code as if final=0.</p></li><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 style="white-space: pre-wrap; word-wrap: break-word;">Remember find_resource() searches a list so it isn't exactly cheap.</p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">for (v = ast_variable_browse(cfg, "modules"); v; v = v->next) {<br>    if (!strcasecmp(v->name, preload_only ? "preload" : "load")) {<br>        if (!preload_only && find_resource(v->value, 0)) {<br>            /* Preload already loaded it */<br>            continue;<br>        }<br>        add_to_load_order(v->value, &load_order, 0);<br>    } else if (!strcasecmp(v->name, preload_only ? "preload-require" : "require")) {<br>        if (!preload_only && find_resource(v->value, 0)) {<br>            /* Preload already loaded it */<br>            continue;<br>        }<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>}</pre></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: 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:19:21 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>