<p>Corey Farrell has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.asterisk.org/6968">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">Modules: Additional improvements to CLI completion.<br><br>Replace 'needsreload' argument with a 'type' argument to specify which<br>type of modules you want completion. This provides more accurate CLI<br>completion for load and unload commands.<br><br>* 'module unload' now excludes modules that have active references or are<br> not running.<br>* 'module load' now excludes modules that are already running.<br>* 'core set debug [atleast] <level> [module]' shows running modules only.<br><br>ASTERISK-27378<br><br>Change-Id: Iea3e00054461484196c46f688f02635cc886bad1<br>---<br>M include/asterisk/module.h<br>M main/cli.c<br>M main/loader.c<br>3 files changed, 133 insertions(+), 52 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/68/6968/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/include/asterisk/module.h b/include/asterisk/module.h<br>index 27c5211..b9d7de6 100644<br>--- a/include/asterisk/module.h<br>+++ b/include/asterisk/module.h<br>@@ -94,6 +94,20 @@<br> AST_MODULE_SUPPORT_DEPRECATED,<br> };<br> <br>+/*! Used to specify which modules should be returned by ast_module_helper. */<br>+enum ast_module_helper_type {<br>+ /*! Modules that are loaded by dlopen. */<br>+ AST_MODULE_HELPER_LOADED = 0,<br>+ /*! Running modules that include a reload callback. */<br>+ AST_MODULE_HELPER_RELOAD = 1,<br>+ /*! Modules that can be loaded or started. */<br>+ AST_MODULE_HELPER_LOAD,<br>+ /*! Modules that can be unloaded. */<br>+ AST_MODULE_HELPER_UNLOAD,<br>+ /*! Running modules */<br>+ AST_MODULE_HELPER_RUNNING,<br>+};<br>+<br> /*! <br> * \brief Load a module.<br> * \param resource_name The name of the module to load.<br>@@ -237,14 +251,13 @@<br> * \param state The possible match to return.<br> * \param rpos The position we should be matching. This should be the same as<br> * pos.<br>- * \param needsreload This should be 1 if we need to reload this module and 0<br>- * otherwise. This function will only return modules that are reloadble<br>- * if this is 1.<br>+ * \param type The type of action that will be performed by CLI.<br>+ * This argument accepts values of enum ast_module_helper_type.<br> *<br> * \retval A possible completion of the partial match.<br> * \retval NULL if no matches were found.<br> */<br>-char *ast_module_helper(const char *line, const char *word, int pos, int state, int rpos, int needsreload);<br>+char *ast_module_helper(const char *line, const char *word, int pos, int state, int rpos, int type);<br> <br> /* Opaque type for module handles generated by the loader */<br> <br>diff --git a/main/cli.c b/main/cli.c<br>index e9ed709..6a5721b 100644<br>--- a/main/cli.c<br>+++ b/main/cli.c<br>@@ -48,7 +48,6 @@<br> #include <regex.h><br> #include <pwd.h><br> #include <grp.h><br>-#include <editline/readline.h><br> <br> #include "asterisk/cli.h"<br> #include "asterisk/linkedlists.h"<br>@@ -226,28 +225,6 @@<br> <br> static AST_RWLIST_HEAD_STATIC(helpers, ast_cli_entry);<br> <br>-static char *complete_fn(const char *word, int state)<br>-{<br>- char *c, *d;<br>- char filename[PATH_MAX];<br>-<br>- if (word[0] == '/')<br>- ast_copy_string(filename, word, sizeof(filename));<br>- else<br>- snprintf(filename, sizeof(filename), "%s/%s", ast_config_AST_MODULE_DIR, word);<br>-<br>- c = d = filename_completion_function(filename, state);<br>-<br>- if (c && word[0] != '/')<br>- c += (strlen(ast_config_AST_MODULE_DIR) + 1);<br>- if (c)<br>- c = ast_strdup(c);<br>-<br>- ast_std_free(d);<br>-<br>- return c;<br>-}<br>-<br> static char *handle_load(struct ast_cli_entry *e, int cmd, struct ast_cli_args *a)<br> {<br> /* "module load <mod>" */<br>@@ -260,12 +237,14 @@<br> return NULL;<br> <br> case CLI_GENERATE:<br>- if (a->pos != e->args)<br>+ if (a->pos != e->args) {<br> return NULL;<br>- return complete_fn(a->word, a->n);<br>+ }<br>+ return ast_module_helper(a->line, a->word, a->pos, a->n, a->pos, AST_MODULE_HELPER_LOAD);<br> }<br>- if (a->argc != e->args + 1)<br>+ if (a->argc != e->args + 1) {<br> return CLI_SHOWUSAGE;<br>+ }<br> if (ast_load_resource(a->argv[e->args])) {<br> ast_cli(a->fd, "Unable to load module %s\n", a->argv[e->args]);<br> return CLI_FAILURE;<br>@@ -288,7 +267,7 @@<br> return NULL;<br> <br> case CLI_GENERATE:<br>- return ast_module_helper(a->line, a->word, a->pos, a->n, a->pos, 1);<br>+ return ast_module_helper(a->line, a->word, a->pos, a->n, a->pos, AST_MODULE_HELPER_RELOAD);<br> }<br> if (a->argc == e->args) {<br> ast_module_reload(NULL);<br>@@ -484,7 +463,7 @@<br> }<br> } else if ((a->pos == 4 && !atleast && strcasecmp(argv3, "off") && strcasecmp(argv3, "channel"))<br> || (a->pos == 5 && atleast)) {<br>- return ast_module_helper(a->line, a->word, a->pos, a->n, a->pos, 0);<br>+ return ast_module_helper(a->line, a->word, a->pos, a->n, a->pos, AST_MODULE_HELPER_RUNNING);<br> }<br> return NULL;<br> }<br>@@ -735,7 +714,7 @@<br> return NULL;<br> <br> case CLI_GENERATE:<br>- return ast_module_helper(a->line, a->word, a->pos, a->n, a->pos, 0);<br>+ return ast_module_helper(a->line, a->word, a->pos, a->n, a->pos, AST_MODULE_HELPER_UNLOAD);<br> }<br> if (a->argc < e->args + 1)<br> return CLI_SHOWUSAGE;<br>@@ -889,10 +868,11 @@<br> return NULL;<br> <br> case CLI_GENERATE:<br>- if (a->pos == e->args)<br>- return ast_module_helper(a->line, a->word, a->pos, a->n, a->pos, 0);<br>- else<br>+ if (a->pos == e->args) {<br>+ return ast_module_helper(a->line, a->word, a->pos, a->n, a->pos, AST_MODULE_HELPER_LOADED);<br>+ } else {<br> return NULL;<br>+ }<br> }<br> /* all the above return, so we proceed with the handler.<br> * we are guaranteed to have argc >= e->args<br>diff --git a/main/loader.c b/main/loader.c<br>index b07efc1..7b7066f 100644<br>--- a/main/loader.c<br>+++ b/main/loader.c<br>@@ -38,6 +38,7 @@<br> #include "asterisk/_private.h"<br> #include "asterisk/paths.h" /* use ast_config_AST_MODULE_DIR */<br> #include <dirent.h><br>+#include <editline/readline.h><br> <br> #include "asterisk/dlinkedlists.h"<br> #include "asterisk/module.h"<br>@@ -710,34 +711,121 @@<br> return res;<br> }<br> <br>-char *ast_module_helper(const char *line, const char *word, int pos, int state, int rpos, int needsreload)<br>+static int module_matches_helper_type(struct ast_module *mod, enum ast_module_helper_type type)<br> {<br>- struct ast_module *cur;<br>- int i, which=0, l = strlen(word);<br>+ switch (type) {<br>+ case AST_MODULE_HELPER_UNLOAD:<br>+ return !mod->usecount && mod->flags.running && !mod->flags.declined;<br>+<br>+ case AST_MODULE_HELPER_RELOAD:<br>+ return mod->flags.running && mod->info->reload;<br>+<br>+ case AST_MODULE_HELPER_RUNNING:<br>+ return mod->flags.running;<br>+<br>+ case AST_MODULE_HELPER_LOADED:<br>+ /* if we have a 'struct ast_module' then we're loaded. */<br>+ return 1;<br>+ default:<br>+ /* This function is not called for AST_MODULE_HELPER_LOAD. */<br>+ /* Unknown ast_module_helper_type. Assume it doesn't match. */<br>+ ast_assert(0);<br>+<br>+ return 0;<br>+ }<br>+}<br>+<br>+static char *module_load_helper(const char *word, int state)<br>+{<br>+ struct ast_module *mod;<br>+ int which = 0;<br>+ char *name;<br>+ char *ret = NULL;<br>+ char *editline_ret;<br>+ char fullpath[PATH_MAX];<br>+ int idx = 0;<br>+ /* This is needed to avoid listing modules that are already running. */<br>+ AST_VECTOR(, char *) running_modules;<br>+<br>+ AST_VECTOR_INIT(&running_modules, 200);<br>+<br>+ AST_DLLIST_LOCK(&module_list);<br>+ AST_DLLIST_TRAVERSE(&module_list, mod, entry) {<br>+ if (mod->flags.running) {<br>+ AST_VECTOR_APPEND(&running_modules, mod->resource);<br>+ }<br>+ }<br>+<br>+ if (word[0] == '/') {<br>+ /* BUGBUG: we should not support this. */<br>+ ast_copy_string(fullpath, word, sizeof(fullpath));<br>+ } else {<br>+ snprintf(fullpath, sizeof(fullpath), "%s/%s", ast_config_AST_MODULE_DIR, word);<br>+ }<br>+<br>+ /*<br>+ * This is ugly that we keep calling filename_completion_function.<br>+ * The only way to avoid this would be to make a copy of the function<br>+ * that skips matches found in the running_modules vector.<br>+ */<br>+ while (!ret && (name = editline_ret = filename_completion_function(fullpath, idx++))) {<br>+ if (word[0] != '/') {<br>+ name += (strlen(ast_config_AST_MODULE_DIR) + 1);<br>+ }<br>+<br>+ /* Don't list files that are already loaded! */<br>+ if (!AST_VECTOR_GET_CMP(&running_modules, name, !strcasecmp) && ++which > state) {<br>+ ret = ast_strdup(name);<br>+ }<br>+<br>+ ast_std_free(editline_ret);<br>+ }<br>+<br>+ /* Do not clean-up the elements, they belong to module_list. */<br>+ AST_VECTOR_FREE(&running_modules);<br>+ AST_DLLIST_UNLOCK(&module_list);<br>+<br>+ return ret;<br>+}<br>+<br>+char *ast_module_helper(const char *line, const char *word, int pos, int state, int rpos, int _type)<br>+{<br>+ enum ast_module_helper_type type = _type;<br>+ struct ast_module *mod;<br>+ int which = 0;<br>+ int wordlen = strlen(word);<br> char *ret = NULL;<br> <br> if (pos != rpos) {<br> return NULL;<br> }<br> <br>+ if (type == AST_MODULE_HELPER_LOAD) {<br>+ return module_load_helper(word, state);<br>+ }<br>+<br>+ if (type == AST_MODULE_HELPER_RELOAD) {<br>+ int idx;<br>+<br>+ for (idx = 0; reload_classes[idx].name; idx++) {<br>+ if (!strncasecmp(word, reload_classes[idx].name, wordlen) && ++which > state) {<br>+ return ast_strdup(reload_classes[idx].name);<br>+ }<br>+ }<br>+ }<br>+<br> AST_DLLIST_LOCK(&module_list);<br>- AST_DLLIST_TRAVERSE(&module_list, cur, entry) {<br>- if (!strncasecmp(word, cur->resource, l) &&<br>- (cur->info->reload || !needsreload) &&<br>- ++which > state) {<br>- ret = ast_strdup(cur->resource);<br>+ AST_DLLIST_TRAVERSE(&module_list, mod, entry) {<br>+ if (!module_matches_helper_type(mod, type)) {<br>+ continue;<br>+ }<br>+<br>+ if (!strncasecmp(word, mod->resource, wordlen) && ++which > state) {<br>+ ret = ast_strdup(mod->resource);<br> break;<br> }<br> }<br> AST_DLLIST_UNLOCK(&module_list);<br>-<br>- if (!ret && needsreload) {<br>- for (i=0; !ret && reload_classes[i].name; i++) {<br>- if (!strncasecmp(word, reload_classes[i].name, l) && ++which > state) {<br>- ret = ast_strdup(reload_classes[i].name);<br>- }<br>- }<br>- }<br> <br> return ret;<br> }<br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/6968">change 6968</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/6968"/><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: newchange </div>
<div style="display:none"> Gerrit-Change-Id: Iea3e00054461484196c46f688f02635cc886bad1 </div>
<div style="display:none"> Gerrit-Change-Number: 6968 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Corey Farrell <git@cfware.com> </div>