<p>Jenkins2 <strong>merged</strong> this change.</p><p><a href="https://gerrit.asterisk.org/8052">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Sean Bright: Looks good to me, but someone else must approve
Joshua Colp: Looks good to me, approved
Jenkins2: Approved for Submit
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">loader: Use ast_cli_completion_add for 'module load' completion.<br><br>This addresses all performance issues with 'module load' completion. In<br>addition to using ast_cli_completion_add we stop using libedit's<br>filename_completion_function, instead using ast_file_read_dir. This<br>ensures all results are produced from a single call to opendir.<br><br>Change-Id: I8bf51ffaa7ef1606f3bd1b5bb13f1905d72c6134<br>---<br>M include/asterisk/file.h<br>M main/Makefile<br>M main/loader.c<br>3 files changed, 54 insertions(+), 50 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/include/asterisk/file.h b/include/asterisk/file.h<br>index 453dc07..c17cb32 100644<br>--- a/include/asterisk/file.h<br>+++ b/include/asterisk/file.h<br>@@ -143,6 +143,11 @@<br> * \param filename the name of the file<br> * \param obj user data object<br> * \return non-zero to stop reading, otherwise zero to continue<br>+ *<br>+ * \note dir_name is not processed by realpath or other functions,<br>+ * symbolic links are not resolved. This ensures dir_name<br>+ * always starts with the exact string originally passed to<br>+ * \ref ast_file_read_dir or \ref ast_file_read_dirs.<br> */<br> typedef int (*ast_file_on_file)(const char *dir_name, const char *filename, void *obj);<br> <br>diff --git a/main/Makefile b/main/Makefile<br>index 4646c1b..bed9199 100644<br>--- a/main/Makefile<br>+++ b/main/Makefile<br>@@ -149,7 +149,6 @@<br> <br> db.o: _ASTCFLAGS+=$(SQLITE3_INCLUDE)<br> asterisk.o: _ASTCFLAGS+=$(LIBEDIT_INCLUDE)<br>-loader.o: _ASTCFLAGS+=$(LIBEDIT_INCLUDE)<br> json.o: _ASTCFLAGS+=$(JANSSON_INCLUDE)<br> bucket.o: _ASTCFLAGS+=$(URIPARSER_INCLUDE)<br> crypt.o: _ASTCFLAGS+=$(CRYPT_INCLUDE)<br>diff --git a/main/loader.c b/main/loader.c<br>index 6119d63..30a9832 100644<br>--- a/main/loader.c<br>+++ b/main/loader.c<br>@@ -38,7 +38,6 @@<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>@@ -58,6 +57,7 @@<br> #include "asterisk/app.h"<br> #include "asterisk/test.h"<br> #include "asterisk/sounds_index.h"<br>+#include "asterisk/cli.h"<br> <br> #include <dlfcn.h><br> <br>@@ -771,57 +771,55 @@<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>+struct module_load_word {<br>+ const char *word;<br>+ size_t len;<br>+ size_t moddir_len;<br>+};<br> <br>- AST_VECTOR_INIT(&running_modules, 200);<br>+static int module_load_helper_on_file(const char *dir_name, const char *filename, void *obj)<br>+{<br>+ struct module_load_word *word = obj;<br>+ struct ast_module *mod;<br>+ char *filename_merged = NULL;<br>+<br>+ /* dir_name will never be shorter than word->moddir_len. */<br>+ dir_name += word->moddir_len;<br>+ if (!ast_strlen_zero(dir_name)) {<br>+ ast_assert(dir_name[0] == '/');<br>+<br>+ dir_name += 1;<br>+ if (ast_asprintf(&filename_merged, "%s/%s", dir_name, filename) < 0) {<br>+ /* If we can't allocate the string just give up! */<br>+ return -1;<br>+ }<br>+ filename = filename_merged;<br>+ }<br>+<br>+ if (!strncasecmp(filename, word->word, word->len)) {<br>+ /* Don't list files that are already loaded! */<br>+ mod = find_resource(filename, 0);<br>+ if (!mod || !mod->flags.running) {<br>+ ast_cli_completion_add(ast_strdup(filename));<br>+ }<br>+ }<br>+<br>+ ast_free(filename_merged);<br>+<br>+ return 0;<br>+}<br>+<br>+static void module_load_helper(const char *word)<br>+{<br>+ struct module_load_word word_l = {<br>+ .word = word,<br>+ .len = strlen(word),<br>+ .moddir_len = strlen(ast_config_AST_MODULE_DIR),<br>+ };<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_file_read_dirs(ast_config_AST_MODULE_DIR, module_load_helper_on_file, &word_l, -1);<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>@@ -837,7 +835,9 @@<br> }<br> <br> if (type == AST_MODULE_HELPER_LOAD) {<br>- return module_load_helper(word, state);<br>+ module_load_helper(word);<br>+<br>+ return NULL;<br> }<br> <br> if (type == AST_MODULE_HELPER_RELOAD) {<br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/8052">change 8052</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/8052"/><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: merged </div>
<div style="display:none"> Gerrit-Change-Id: I8bf51ffaa7ef1606f3bd1b5bb13f1905d72c6134 </div>
<div style="display:none"> Gerrit-Change-Number: 8052 </div>
<div style="display:none"> Gerrit-PatchSet: 4 </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: Jenkins2 </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Richard Mudgett <rmudgett@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Sean Bright <sean.bright@gmail.com> </div>