[Asterisk-code-review] Modules: Additional improvements to CLI completion. (asterisk[15])

Corey Farrell asteriskteam at digium.com
Thu Nov 2 07:30:41 CDT 2017


Corey Farrell has uploaded this change for review. ( https://gerrit.asterisk.org/6967


Change subject: Modules: Additional improvements to CLI completion.
......................................................................

Modules: Additional improvements to CLI completion.

Replace 'needsreload' argument with a 'type' argument to specify which
type of modules you want completion.  This provides more accurate CLI
completion for load and unload commands.

* 'module unload' now excludes modules that have active references or are
  not running.
* 'module load' now excludes modules that are already running.
* 'core set debug [atleast] <level> [module]' shows running modules only.

ASTERISK-27378

Change-Id: Iea3e00054461484196c46f688f02635cc886bad1
---
M include/asterisk/module.h
M main/cli.c
M main/loader.c
3 files changed, 133 insertions(+), 52 deletions(-)



  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/67/6967/1

diff --git a/include/asterisk/module.h b/include/asterisk/module.h
index e614a72..6c60429 100644
--- a/include/asterisk/module.h
+++ b/include/asterisk/module.h
@@ -94,6 +94,20 @@
 	AST_MODULE_SUPPORT_DEPRECATED,
 };
 
+/*! Used to specify which modules should be returned by ast_module_helper. */
+enum ast_module_helper_type {
+	/*! Modules that are loaded by dlopen. */
+	AST_MODULE_HELPER_LOADED = 0,
+	/*! Running modules that include a reload callback. */
+	AST_MODULE_HELPER_RELOAD = 1,
+	/*! Modules that can be loaded or started. */
+	AST_MODULE_HELPER_LOAD,
+	/*! Modules that can be unloaded. */
+	AST_MODULE_HELPER_UNLOAD,
+	/*! Running modules */
+	AST_MODULE_HELPER_RUNNING,
+};
+
 /*! 
  * \brief Load a module.
  * \param resource_name The name of the module to load.
@@ -237,14 +251,13 @@
  * \param state The possible match to return.
  * \param rpos The position we should be matching.  This should be the same as
  *        pos.
- * \param needsreload This should be 1 if we need to reload this module and 0
- *        otherwise.  This function will only return modules that are reloadble
- *        if this is 1.
+ * \param type The type of action that will be performed by CLI.
+ *             This argument accepts values of enum ast_module_helper_type.
  *
  * \retval A possible completion of the partial match.
  * \retval NULL if no matches were found.
  */
-char *ast_module_helper(const char *line, const char *word, int pos, int state, int rpos, int needsreload);
+char *ast_module_helper(const char *line, const char *word, int pos, int state, int rpos, int type);
 
 /* Opaque type for module handles generated by the loader */
 
diff --git a/main/cli.c b/main/cli.c
index d9aab85..0896181 100644
--- a/main/cli.c
+++ b/main/cli.c
@@ -45,7 +45,6 @@
 #include <regex.h>
 #include <pwd.h>
 #include <grp.h>
-#include <editline/readline.h>
 
 #include "asterisk/cli.h"
 #include "asterisk/linkedlists.h"
@@ -224,28 +223,6 @@
 
 static AST_RWLIST_HEAD_STATIC(helpers, ast_cli_entry);
 
-static char *complete_fn(const char *word, int state)
-{
-	char *c, *d;
-	char filename[PATH_MAX];
-
-	if (word[0] == '/')
-		ast_copy_string(filename, word, sizeof(filename));
-	else
-		snprintf(filename, sizeof(filename), "%s/%s", ast_config_AST_MODULE_DIR, word);
-
-	c = d = filename_completion_function(filename, state);
-
-	if (c && word[0] != '/')
-		c += (strlen(ast_config_AST_MODULE_DIR) + 1);
-	if (c)
-		c = ast_strdup(c);
-
-	ast_std_free(d);
-
-	return c;
-}
-
 static char *handle_load(struct ast_cli_entry *e, int cmd, struct ast_cli_args *a)
 {
 	/* "module load <mod>" */
@@ -258,12 +235,14 @@
 		return NULL;
 
 	case CLI_GENERATE:
-		if (a->pos != e->args)
+		if (a->pos != e->args) {
 			return NULL;
-		return complete_fn(a->word, a->n);
+		}
+		return ast_module_helper(a->line, a->word, a->pos, a->n, a->pos, AST_MODULE_HELPER_LOAD);
 	}
-	if (a->argc != e->args + 1)
+	if (a->argc != e->args + 1) {
 		return CLI_SHOWUSAGE;
+	}
 	if (ast_load_resource(a->argv[e->args])) {
 		ast_cli(a->fd, "Unable to load module %s\n", a->argv[e->args]);
 		return CLI_FAILURE;
@@ -286,7 +265,7 @@
 		return NULL;
 
 	case CLI_GENERATE:
-		return ast_module_helper(a->line, a->word, a->pos, a->n, a->pos, 1);
+		return ast_module_helper(a->line, a->word, a->pos, a->n, a->pos, AST_MODULE_HELPER_RELOAD);
 	}
 	if (a->argc == e->args) {
 		ast_module_reload(NULL);
@@ -482,7 +461,7 @@
 			}
 		} else if ((a->pos == 4 && !atleast && strcasecmp(argv3, "off") && strcasecmp(argv3, "channel"))
 			|| (a->pos == 5 && atleast)) {
-			return ast_module_helper(a->line, a->word, a->pos, a->n, a->pos, 0);
+			return ast_module_helper(a->line, a->word, a->pos, a->n, a->pos, AST_MODULE_HELPER_RUNNING);
 		}
 		return NULL;
 	}
@@ -733,7 +712,7 @@
 		return NULL;
 
 	case CLI_GENERATE:
-		return ast_module_helper(a->line, a->word, a->pos, a->n, a->pos, 0);
+		return ast_module_helper(a->line, a->word, a->pos, a->n, a->pos, AST_MODULE_HELPER_UNLOAD);
 	}
 	if (a->argc < e->args + 1)
 		return CLI_SHOWUSAGE;
@@ -889,10 +868,11 @@
 		return NULL;
 
 	case CLI_GENERATE:
-		if (a->pos == e->args)
-			return ast_module_helper(a->line, a->word, a->pos, a->n, a->pos, 0);
-		else
+		if (a->pos == e->args) {
+			return ast_module_helper(a->line, a->word, a->pos, a->n, a->pos, AST_MODULE_HELPER_LOADED);
+		} else {
 			return NULL;
+		}
 	}
 	/* all the above return, so we proceed with the handler.
 	 * we are guaranteed to have argc >= e->args
diff --git a/main/loader.c b/main/loader.c
index 8250f1f..fc3e9a3 100644
--- a/main/loader.c
+++ b/main/loader.c
@@ -36,6 +36,7 @@
 #include "asterisk/_private.h"
 #include "asterisk/paths.h"	/* use ast_config_AST_MODULE_DIR */
 #include <dirent.h>
+#include <editline/readline.h>
 
 #include "asterisk/dlinkedlists.h"
 #include "asterisk/module.h"
@@ -702,34 +703,121 @@
 	return res;
 }
 
-char *ast_module_helper(const char *line, const char *word, int pos, int state, int rpos, int needsreload)
+static int module_matches_helper_type(struct ast_module *mod, enum ast_module_helper_type type)
 {
-	struct ast_module *cur;
-	int i, which=0, l = strlen(word);
+	switch (type) {
+	case AST_MODULE_HELPER_UNLOAD:
+		return !mod->usecount && mod->flags.running && !mod->flags.declined;
+
+	case AST_MODULE_HELPER_RELOAD:
+		return mod->flags.running && mod->info->reload;
+
+	case AST_MODULE_HELPER_RUNNING:
+		return mod->flags.running;
+
+	case AST_MODULE_HELPER_LOADED:
+		/* if we have a 'struct ast_module' then we're loaded. */
+		return 1;
+	default:
+		/* This function is not called for AST_MODULE_HELPER_LOAD. */
+		/* Unknown ast_module_helper_type. Assume it doesn't match. */
+		ast_assert(0);
+
+		return 0;
+	}
+}
+
+static char *module_load_helper(const char *word, int state)
+{
+	struct ast_module *mod;
+	int which = 0;
+	char *name;
+	char *ret = NULL;
+	char *editline_ret;
+	char fullpath[PATH_MAX];
+	int idx = 0;
+	/* This is needed to avoid listing modules that are already running. */
+	AST_VECTOR(, char *) running_modules;
+
+	AST_VECTOR_INIT(&running_modules, 200);
+
+	AST_DLLIST_LOCK(&module_list);
+	AST_DLLIST_TRAVERSE(&module_list, mod, entry) {
+		if (mod->flags.running) {
+			AST_VECTOR_APPEND(&running_modules, mod->resource);
+		}
+	}
+
+	if (word[0] == '/') {
+		/* BUGBUG: we should not support this. */
+		ast_copy_string(fullpath, word, sizeof(fullpath));
+	} else {
+		snprintf(fullpath, sizeof(fullpath), "%s/%s", ast_config_AST_MODULE_DIR, word);
+	}
+
+	/*
+	 * This is ugly that we keep calling filename_completion_function.
+	 * The only way to avoid this would be to make a copy of the function
+	 * that skips matches found in the running_modules vector.
+	 */
+	while (!ret && (name = editline_ret = filename_completion_function(fullpath, idx++))) {
+		if (word[0] != '/') {
+			name += (strlen(ast_config_AST_MODULE_DIR) + 1);
+		}
+
+		/* Don't list files that are already loaded! */
+		if (!AST_VECTOR_GET_CMP(&running_modules, name, !strcasecmp) && ++which > state) {
+			ret = ast_strdup(name);
+		}
+
+		ast_std_free(editline_ret);
+	}
+
+	/* Do not clean-up the elements, they belong to module_list. */
+	AST_VECTOR_FREE(&running_modules);
+	AST_DLLIST_UNLOCK(&module_list);
+
+	return ret;
+}
+
+char *ast_module_helper(const char *line, const char *word, int pos, int state, int rpos, int _type)
+{
+	enum ast_module_helper_type type = _type;
+	struct ast_module *mod;
+	int which = 0;
+	int wordlen = strlen(word);
 	char *ret = NULL;
 
 	if (pos != rpos) {
 		return NULL;
 	}
 
+	if (type == AST_MODULE_HELPER_LOAD) {
+		return module_load_helper(word, state);
+	}
+
+	if (type == AST_MODULE_HELPER_RELOAD) {
+		int idx;
+
+		for (idx = 0; reload_classes[idx].name; idx++) {
+			if (!strncasecmp(word, reload_classes[idx].name, wordlen) && ++which > state) {
+				return ast_strdup(reload_classes[idx].name);
+			}
+		}
+	}
+
 	AST_DLLIST_LOCK(&module_list);
-	AST_DLLIST_TRAVERSE(&module_list, cur, entry) {
-		if (!strncasecmp(word, cur->resource, l) &&
-		    (cur->info->reload || !needsreload) &&
-		    ++which > state) {
-			ret = ast_strdup(cur->resource);
+	AST_DLLIST_TRAVERSE(&module_list, mod, entry) {
+		if (!module_matches_helper_type(mod, type)) {
+			continue;
+		}
+
+		if (!strncasecmp(word, mod->resource, wordlen) && ++which > state) {
+			ret = ast_strdup(mod->resource);
 			break;
 		}
 	}
 	AST_DLLIST_UNLOCK(&module_list);
-
-	if (!ret && needsreload) {
-		for (i=0; !ret && reload_classes[i].name; i++) {
-			if (!strncasecmp(word, reload_classes[i].name, l) && ++which > state) {
-				ret = ast_strdup(reload_classes[i].name);
-			}
-		}
-	}
 
 	return ret;
 }

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

Gerrit-Project: asterisk
Gerrit-Branch: 15
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iea3e00054461484196c46f688f02635cc886bad1
Gerrit-Change-Number: 6967
Gerrit-PatchSet: 1
Gerrit-Owner: Corey Farrell <git at cfware.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20171102/54bc8758/attachment-0001.html>


More information about the asterisk-code-review mailing list