[Asterisk-code-review] loader: Use ast cli completion add for 'module load' complet... (asterisk[master])
Jenkins2
asteriskteam at digium.com
Wed Jan 31 07:48:22 CST 2018
Jenkins2 has submitted this change and it was merged. ( https://gerrit.asterisk.org/8050 )
Change subject: loader: Use ast_cli_completion_add for 'module load' completion.
......................................................................
loader: Use ast_cli_completion_add for 'module load' completion.
This addresses all performance issues with 'module load' completion. In
addition to using ast_cli_completion_add we stop using libedit's
filename_completion_function, instead using ast_file_read_dir. This
ensures all results are produced from a single call to opendir.
Change-Id: I8bf51ffaa7ef1606f3bd1b5bb13f1905d72c6134
---
M include/asterisk/file.h
M main/Makefile
M main/loader.c
3 files changed, 54 insertions(+), 50 deletions(-)
Approvals:
Sean Bright: Looks good to me, but someone else must approve
Joshua Colp: Looks good to me, approved
Jenkins2: Approved for Submit
diff --git a/include/asterisk/file.h b/include/asterisk/file.h
index 453dc07..c17cb32 100644
--- a/include/asterisk/file.h
+++ b/include/asterisk/file.h
@@ -143,6 +143,11 @@
* \param filename the name of the file
* \param obj user data object
* \return non-zero to stop reading, otherwise zero to continue
+ *
+ * \note dir_name is not processed by realpath or other functions,
+ * symbolic links are not resolved. This ensures dir_name
+ * always starts with the exact string originally passed to
+ * \ref ast_file_read_dir or \ref ast_file_read_dirs.
*/
typedef int (*ast_file_on_file)(const char *dir_name, const char *filename, void *obj);
diff --git a/main/Makefile b/main/Makefile
index b148b6f..7e9624e 100644
--- a/main/Makefile
+++ b/main/Makefile
@@ -154,7 +154,6 @@
db.o: _ASTCFLAGS+=$(SQLITE3_INCLUDE)
asterisk.o: _ASTCFLAGS+=$(LIBEDIT_INCLUDE)
-loader.o: _ASTCFLAGS+=$(LIBEDIT_INCLUDE)
json.o: _ASTCFLAGS+=$(JANSSON_INCLUDE)
bucket.o: _ASTCFLAGS+=$(URIPARSER_INCLUDE)
crypt.o: _ASTCFLAGS+=$(CRYPT_INCLUDE)
diff --git a/main/loader.c b/main/loader.c
index da508f3..e664a23 100644
--- a/main/loader.c
+++ b/main/loader.c
@@ -36,7 +36,6 @@
#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"
@@ -56,6 +55,7 @@
#include "asterisk/app.h"
#include "asterisk/test.h"
#include "asterisk/sounds_index.h"
+#include "asterisk/cli.h"
#include <dlfcn.h>
@@ -1036,57 +1036,55 @@
}
}
-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;
+struct module_load_word {
+ const char *word;
+ size_t len;
+ size_t moddir_len;
+};
- AST_VECTOR_INIT(&running_modules, 200);
+static int module_load_helper_on_file(const char *dir_name, const char *filename, void *obj)
+{
+ struct module_load_word *word = obj;
+ struct ast_module *mod;
+ char *filename_merged = NULL;
+
+ /* dir_name will never be shorter than word->moddir_len. */
+ dir_name += word->moddir_len;
+ if (!ast_strlen_zero(dir_name)) {
+ ast_assert(dir_name[0] == '/');
+
+ dir_name += 1;
+ if (ast_asprintf(&filename_merged, "%s/%s", dir_name, filename) < 0) {
+ /* If we can't allocate the string just give up! */
+ return -1;
+ }
+ filename = filename_merged;
+ }
+
+ if (!strncasecmp(filename, word->word, word->len)) {
+ /* Don't list files that are already loaded! */
+ mod = find_resource(filename, 0);
+ if (!mod || !mod->flags.running) {
+ ast_cli_completion_add(ast_strdup(filename));
+ }
+ }
+
+ ast_free(filename_merged);
+
+ return 0;
+}
+
+static void module_load_helper(const char *word)
+{
+ struct module_load_word word_l = {
+ .word = word,
+ .len = strlen(word),
+ .moddir_len = strlen(ast_config_AST_MODULE_DIR),
+ };
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_file_read_dirs(ast_config_AST_MODULE_DIR, module_load_helper_on_file, &word_l, -1);
AST_DLLIST_UNLOCK(&module_list);
-
- return ret;
}
char *ast_module_helper(const char *line, const char *word, int pos, int state, int rpos, enum ast_module_helper_type type)
@@ -1101,7 +1099,9 @@
}
if (type == AST_MODULE_HELPER_LOAD) {
- return module_load_helper(word, state);
+ module_load_helper(word);
+
+ return NULL;
}
if (type == AST_MODULE_HELPER_RELOAD) {
--
To view, visit https://gerrit.asterisk.org/8050
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I8bf51ffaa7ef1606f3bd1b5bb13f1905d72c6134
Gerrit-Change-Number: 8050
Gerrit-PatchSet: 4
Gerrit-Owner: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Sean Bright <sean.bright at gmail.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180131/709c80a2/attachment.html>
More information about the asterisk-code-review
mailing list