<p>Corey Farrell has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.asterisk.org/6996">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">CLI: Improve completion performance.<br><br>This patch contains several improvements to CLI completion:<br>* Remote consoles no longer ask for the number of matches before asking<br> for the list. Doing so caused double the work to be done.<br>* For functions that deal with the list of matches replace 'char **'<br> with a vector of 'char *'. The structure is very similar but the<br> vector has macro's to assist with building the list and the vector<br> keeps count.<br>* Add ast_cli_completion_vector_save to support CLI handlers providing<br> all completion options in a single response to request for completion.<br> Using this method is significantly faster. It should be considered<br> for CLI completions that require locking or iterating large<br> containers.<br>* Deprecate ast_cli_generatornummatches and ast_cli_completion_matches.<br>* Update ast_module_helper to use ast_cli_completion_vector_save.<br><br>Change-Id: Ie527d3d5e8efcf5eaddf882db8b1f904e86543a8<br>---<br>M include/asterisk/cli.h<br>M include/asterisk/vector.h<br>M main/asterisk.c<br>M main/cli.c<br>M main/loader.c<br>M tests/test_substitution.c<br>6 files changed, 391 insertions(+), 277 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/96/6996/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/include/asterisk/cli.h b/include/asterisk/cli.h<br>index c79a4e9..0e3e298 100644<br>--- a/include/asterisk/cli.h<br>+++ b/include/asterisk/cli.h<br>@@ -287,9 +287,18 @@<br> * Useful for readline, that's about it<br> * \retval 0 on success<br> * \retval -1 on failure<br>+ *<br>+ * \note This function should not be run in a loop. It is<br>+ * much more efficient to use ast_cli_completion_vector<br>+ * if you need the full list.<br> */<br> char *ast_cli_generator(const char *, const char *, int);<br> <br>+/*!<br>+ * \brief Determine how many completion matches are possible.<br>+ *<br>+ * \deprecated Use \ref ast_cli_completion_vector.<br>+ */<br> int ast_cli_generatornummatches(const char *, const char *);<br> <br> /*!<br>@@ -302,10 +311,41 @@<br> * Subsequent entries are all possible values, followed by a NULL.<br> * All strings and the array itself are malloc'ed and must be freed<br> * by the caller.<br>+ *<br>+ * \deprecated Use \ref ast_cli_completion_vector.<br> */<br> char **ast_cli_completion_matches(const char *, const char *);<br> <br> /*!<br>+ * \brief Generates a vector of strings for CLI completion.<br>+ *<br>+ * \param text Complete input being matched.<br>+ * \param word Current word being matched<br>+ *<br>+ * The results contain strings that both:<br>+ * 1) Begin with the string in \a word.<br>+ * 2) Are valid in a command after the string in \a text.<br>+ *<br>+ * The first entry (offset 0) of the result is the longest common substring<br>+ * in the results, useful to extend the string that has been completed.<br>+ * Subsequent entries are all possible values.<br>+ *<br>+ * \note All strings and the vector itself are malloc'ed and must be freed<br>+ * by the caller.<br>+ *<br>+ * \note The vector is sorted and does not contain any duplicates.<br>+ */<br>+struct ast_vector_string *ast_cli_completion_vector(const char *text, const char *word);<br>+<br>+/*!<br>+ * \brief Provide all CLI completion options at once.<br>+ *<br>+ * This is to be used in response to CLI_GENERATE for the first word,<br>+ * NULL must be returned by the CLI function for this to work.<br>+ */<br>+void ast_cli_completion_vector_save(struct ast_vector_string *matches);<br>+<br>+/*!<br> * \brief Command completion for the list of active channels.<br> *<br> * This can be called from a CLI command completion function that wants to<br>diff --git a/include/asterisk/vector.h b/include/asterisk/vector.h<br>index 68ce130..384745f 100644<br>--- a/include/asterisk/vector.h<br>+++ b/include/asterisk/vector.h<br>@@ -51,6 +51,9 @@<br> /*! \brief Integer vector definition */<br> AST_VECTOR(ast_vector_int, int);<br> <br>+/*! \brief String vector definition */<br>+AST_VECTOR(ast_vector_string, char *);<br>+<br> /*!<br> * \brief Define a vector structure with a read/write lock<br> *<br>@@ -88,6 +91,24 @@<br> (vec)->max = 0; \<br> } \<br> (alloc_size == 0 || (vec)->elems != NULL) ? 0 : -1; \<br>+})<br>+<br>+/*!<br>+ * \brief Steal the elements from a vector and reinitialize.<br>+ *<br>+ * \param vec Vector to operate on.<br>+ *<br>+ * This allows you to use vector.h to construct a list and use the<br>+ * data as a bare array.<br>+ *<br>+ * \warning AST_VECTOR_SIZE and AST_VECTOR_MAX_SIZE are both reset<br>+ * to 0. If either are needed they must be saved to a local<br>+ * variable before stealing the elements.<br>+ */<br>+#define AST_VECTOR_STEAL_ELEMENTS(vec) ({ \<br>+ typeof((vec)->elems) __elems = (vec)->elems; \<br>+ AST_VECTOR_INIT((vec), 0); \<br>+ (__elems); \<br> })<br> <br> /*!<br>@@ -192,6 +213,25 @@<br> })<br> <br> /*!<br>+ * \brief Run 'qsort' against the vector elements.<br>+ *<br>+ * \param vec Vector to sort.<br>+ * \param qsortcmp The compare callback function to be used by qsort.<br>+ *<br>+ * \note qsortcmp must be a real function that can be passed by address<br>+ * to qsort.<br>+ *<br>+ * \note qsort operates on pointers to the element's storage in the vector.<br>+ * Each argument given to qsortcmp is the pointer to the element in<br>+ * the vector like what is returned by AST_VECTOR_GET_ADDR.<br>+ */<br>+#define AST_VECTOR_QSORT(vec, qsortcmp) do { \<br>+ if (vec && vec->current) { \<br>+ qsort(vec->elems, vec->current, sizeof(vec->elems[0]), qsortcmp); \<br>+ } \<br>+} while(0)<br>+<br>+/*!<br> * \brief Append an element to a vector, growing the vector if needed.<br> *<br> * \param vec Vector to append to.<br>diff --git a/main/asterisk.c b/main/asterisk.c<br>index d555949..54956eb 100644<br>--- a/main/asterisk.c<br>+++ b/main/asterisk.c<br>@@ -3016,121 +3016,70 @@<br> return ast_str_buffer(prompt);<br> }<br> <br>-static void destroy_match_list(char **match_list, int matches)<br>-{<br>- if (match_list) {<br>- int idx;<br>-<br>- for (idx = 0; idx < matches; ++idx) {<br>- ast_free(match_list[idx]);<br>- }<br>- ast_free(match_list);<br>- }<br>-}<br>-<br>-static char **ast_el_strtoarr(char *buf)<br>+static struct ast_vector_string *ast_el_strtoarr(char *buf)<br> {<br> char *retstr;<br>- char **match_list = NULL;<br>- char **new_list;<br>- size_t match_list_len = 1;<br>- int matches = 0;<br>+ struct ast_vector_string *vec = ast_calloc(1, sizeof(*vec));<br>+<br>+ if (!vec) {<br>+ return NULL;<br>+ }<br> <br> while ((retstr = strsep(&buf, " "))) {<br> if (!strcmp(retstr, AST_CLI_COMPLETE_EOF)) {<br> break;<br> }<br>- if (matches + 1 >= match_list_len) {<br>- match_list_len <<= 1;<br>- new_list = ast_realloc(match_list, match_list_len * sizeof(char *));<br>- if (!new_list) {<br>- destroy_match_list(match_list, matches);<br>- return NULL;<br>- }<br>- match_list = new_list;<br>- }<br> <br> retstr = ast_strdup(retstr);<br>- if (!retstr) {<br>- destroy_match_list(match_list, matches);<br>- return NULL;<br>+ if (!retstr || AST_VECTOR_APPEND(vec, retstr)) {<br>+ ast_free(retstr);<br>+ goto vector_cleanup;<br> }<br>- match_list[matches++] = retstr;<br> }<br> <br>- if (!match_list) {<br>- return NULL;<br>+ if (!AST_VECTOR_SIZE(vec)) {<br>+ goto vector_cleanup;<br> }<br> <br>- if (matches >= match_list_len) {<br>- new_list = ast_realloc(match_list, (match_list_len + 1) * sizeof(char *));<br>- if (!new_list) {<br>- destroy_match_list(match_list, matches);<br>- return NULL;<br>- }<br>- match_list = new_list;<br>- }<br>+ return vec;<br> <br>- match_list[matches] = NULL;<br>+vector_cleanup:<br>+ AST_VECTOR_CALLBACK_VOID(vec, ast_free);<br>+ AST_VECTOR_PTR_FREE(vec);<br> <br>- return match_list;<br>+ return NULL;<br> }<br> <br>-static int ast_el_sort_compare(const void *i1, const void *i2)<br>+static int ast_cli_display_match_list(struct ast_vector_string *matches, int max)<br> {<br>- char *s1, *s2;<br>-<br>- s1 = ((char **)i1)[0];<br>- s2 = ((char **)i2)[0];<br>-<br>- return strcasecmp(s1, s2);<br>-}<br>-<br>-static int ast_cli_display_match_list(char **matches, int len, int max)<br>-{<br>- int i, idx, limit, count;<br>- int screenwidth = 0;<br>- int numoutput = 0, numoutputline = 0;<br>-<br>- screenwidth = ast_get_termcols(STDOUT_FILENO);<br>+ int i;<br>+ int idx;<br>+ int limit;<br>+ int screenwidth = ast_get_termcols(STDOUT_FILENO);<br>+ int numoutputline = 0;<br> <br> /* find out how many entries can be put on one line, with two spaces between strings */<br> limit = screenwidth / (max + 2);<br>- if (limit == 0)<br>+ if (limit == 0) {<br> limit = 1;<br>-<br>- /* how many lines of output */<br>- count = len / limit;<br>- if (count * limit < len)<br>- count++;<br>+ }<br> <br> idx = 1;<br> <br>- qsort(&matches[0], (size_t)(len), sizeof(char *), ast_el_sort_compare);<br>-<br>- for (; count > 0; count--) {<br>+ do {<br> numoutputline = 0;<br>- for (i = 0; i < limit && matches[idx]; i++, idx++) {<br> <br>- /* Don't print dupes */<br>- if ( (matches[idx+1] != NULL && strcmp(matches[idx], matches[idx+1]) == 0 ) ) {<br>- i--;<br>- ast_free(matches[idx]);<br>- matches[idx] = NULL;<br>- continue;<br>- }<br>-<br>- numoutput++;<br>+ for (i = 0; i < limit && idx < AST_VECTOR_SIZE(matches); i++, idx++) {<br> numoutputline++;<br>- fprintf(stdout, "%-*s ", max, matches[idx]);<br>- ast_free(matches[idx]);<br>- matches[idx] = NULL;<br>+ fprintf(stdout, "%-*s ", max, AST_VECTOR_GET(matches, idx));<br> }<br>- if (numoutputline > 0)<br>- fprintf(stdout, "\n");<br>- }<br> <br>- return numoutput;<br>+ if (numoutputline > 0) {<br>+ fprintf(stdout, "\n");<br>+ }<br>+ } while (numoutputline);<br>+<br>+ return AST_VECTOR_SIZE(matches) - 1;<br> }<br> <br> <br>@@ -3138,8 +3087,7 @@<br> {<br> int len = 0;<br> char *ptr;<br>- int nummatches = 0;<br>- char **matches;<br>+ struct ast_vector_string *matches;<br> int retval = CC_ERROR;<br> char buf[2048], savechr;<br> int res;<br>@@ -3162,96 +3110,80 @@<br> len = lf->cursor - ptr;<br> <br> if (ast_opt_remote) {<br>- snprintf(buf, sizeof(buf), "_COMMAND NUMMATCHES \"%s\" \"%s\"", lf->buffer, ptr);<br>+ char *mbuf;<br>+ char *new_mbuf;<br>+ int mlen = 0, maxmbuf = 2048;<br>+<br>+ /* Start with a 2048 byte buffer */<br>+ if (!(mbuf = ast_malloc(maxmbuf))) {<br>+ *((char *) lf->cursor) = savechr;<br>+<br>+ return (char *)(CC_ERROR);<br>+ }<br>+ snprintf(buf, sizeof(buf), "_COMMAND MATCHESARRAY \"%s\" \"%s\"", lf->buffer, ptr);<br> fdsend(ast_consock, buf);<br>- if ((res = read(ast_consock, buf, sizeof(buf) - 1)) < 0) {<br>- return (char*)(CC_ERROR);<br>- }<br>- buf[res] = '\0';<br>- nummatches = atoi(buf);<br>+ res = 0;<br>+ mbuf[0] = '\0';<br> <br>- if (nummatches > 0) {<br>- char *mbuf;<br>- char *new_mbuf;<br>- int mlen = 0, maxmbuf = 2048;<br>+ while (!strstr(mbuf, AST_CLI_COMPLETE_EOF) && res != -1) {<br>+ if (mlen + 1024 > maxmbuf) {<br>+ /* Every step increment buffer 1024 bytes */<br>+ maxmbuf += 1024;<br>+ new_mbuf = ast_realloc(mbuf, maxmbuf);<br>+ if (!new_mbuf) {<br>+ ast_free(mbuf);<br>+ *((char *) lf->cursor) = savechr;<br> <br>- /* Start with a 2048 byte buffer */<br>- if (!(mbuf = ast_malloc(maxmbuf))) {<br>- *((char *) lf->cursor) = savechr;<br>- return (char *)(CC_ERROR);<br>- }<br>- snprintf(buf, sizeof(buf), "_COMMAND MATCHESARRAY \"%s\" \"%s\"", lf->buffer, ptr);<br>- fdsend(ast_consock, buf);<br>- res = 0;<br>- mbuf[0] = '\0';<br>- while (!strstr(mbuf, AST_CLI_COMPLETE_EOF) && res != -1) {<br>- if (mlen + 1024 > maxmbuf) {<br>- /* Every step increment buffer 1024 bytes */<br>- maxmbuf += 1024;<br>- new_mbuf = ast_realloc(mbuf, maxmbuf);<br>- if (!new_mbuf) {<br>- ast_free(mbuf);<br>- *((char *) lf->cursor) = savechr;<br>- return (char *)(CC_ERROR);<br>- }<br>- mbuf = new_mbuf;<br>+ return (char *)(CC_ERROR);<br> }<br>- /* Only read 1024 bytes at a time */<br>- res = read(ast_consock, mbuf + mlen, 1024);<br>- if (res > 0)<br>- mlen += res;<br>+ mbuf = new_mbuf;<br> }<br>- mbuf[mlen] = '\0';<br>-<br>- matches = ast_el_strtoarr(mbuf);<br>- ast_free(mbuf);<br>- } else<br>- matches = (char **) NULL;<br>- } else {<br>- char **p, *oldbuf=NULL;<br>- nummatches = 0;<br>- matches = ast_cli_completion_matches((char *)lf->buffer,ptr);<br>- for (p = matches; p && *p; p++) {<br>- if (!oldbuf || strcmp(*p,oldbuf))<br>- nummatches++;<br>- oldbuf = *p;<br>+ /* Only read 1024 bytes at a time */<br>+ res = read(ast_consock, mbuf + mlen, 1024);<br>+ if (res > 0) {<br>+ mlen += res;<br>+ }<br> }<br>+ mbuf[mlen] = '\0';<br>+<br>+ matches = ast_el_strtoarr(mbuf);<br>+ ast_free(mbuf);<br>+ } else {<br>+ matches = ast_cli_completion_vector((char *)lf->buffer, ptr);<br> }<br> <br> if (matches) {<br> int i;<br>- int matches_num, maxlen, match_len;<br>+ char *best_match = AST_VECTOR_GET(matches, 0);<br> <br>- if (matches[0][0] != '\0') {<br>+ if (!ast_strlen_zero(best_match)) {<br> el_deletestr(editline, (int) len);<br>- el_insertstr(editline, matches[0]);<br>+ el_insertstr(editline, best_match);<br> retval = CC_REFRESH;<br> }<br> <br>- if (nummatches == 1) {<br>+ if (AST_VECTOR_SIZE(matches) == 2) {<br> /* Found an exact match */<br> el_insertstr(editline, " ");<br> retval = CC_REFRESH;<br> } else {<br> /* Must be more than one match */<br>- for (i = 1, maxlen = 0; matches[i]; i++) {<br>- match_len = strlen(matches[i]);<br>- if (match_len > maxlen)<br>+ int maxlen = 0;<br>+<br>+ for (i = 1; i < AST_VECTOR_SIZE(matches); i++) {<br>+ int match_len = strlen(AST_VECTOR_GET(matches, i));<br>+ if (match_len > maxlen) {<br> maxlen = match_len;<br>+ }<br> }<br>- matches_num = i - 1;<br>- if (matches_num >1) {<br>- fprintf(stdout, "\n");<br>- ast_cli_display_match_list(matches, nummatches, maxlen);<br>- retval = CC_REDISPLAY;<br>- } else {<br>- el_insertstr(editline," ");<br>- retval = CC_REFRESH;<br>- }<br>+<br>+ fprintf(stdout, "\n");<br>+ ast_cli_display_match_list(matches, maxlen);<br>+ retval = CC_REDISPLAY;<br> }<br>- for (i = 0; matches[i]; i++)<br>- ast_free(matches[i]);<br>- ast_free(matches);<br>+<br>+ AST_VECTOR_CALLBACK_VOID(matches, ast_free);<br>+ AST_VECTOR_PTR_FREE(matches);<br> }<br> <br> *((char *) lf->cursor) = savechr;<br>diff --git a/main/cli.c b/main/cli.c<br>index 0896181..c8d4b91 100644<br>--- a/main/cli.c<br>+++ b/main/cli.c<br>@@ -1283,11 +1283,9 @@<br> <br> static char *handle_commandmatchesarray(struct ast_cli_entry *e, int cmd, struct ast_cli_args *a)<br> {<br>- char *buf, *obuf;<br>- int buflen = 2048;<br>- int len = 0;<br>- char **matches;<br>- int x, matchlen;<br>+ char *buf;<br>+ struct ast_vector_string *matches;<br>+ int x;<br> <br> switch (cmd) {<br> case CLI_INIT:<br>@@ -1303,33 +1301,35 @@<br> <br> if (a->argc != 4)<br> return CLI_SHOWUSAGE;<br>- if (!(buf = ast_malloc(buflen)))<br>- return CLI_FAILURE;<br>- buf[len] = '\0';<br>- matches = ast_cli_completion_matches(a->argv[2], a->argv[3]);<br>+<br>+ matches = ast_cli_completion_vector(a->argv[2], a->argv[3]);<br> if (matches) {<br>- for (x=0; matches[x]; x++) {<br>- matchlen = strlen(matches[x]) + 1;<br>- if (len + matchlen >= buflen) {<br>- buflen += matchlen * 3;<br>- obuf = buf;<br>- if (!(buf = ast_realloc(obuf, buflen)))<br>- /* Memory allocation failure... Just free old buffer and be done */<br>- ast_free(obuf);<br>- }<br>- if (buf)<br>- len += sprintf( buf + len, "%s ", matches[x]);<br>- ast_free(matches[x]);<br>- matches[x] = NULL;<br>+ int len = 0;<br>+<br>+ for (x = 0; x < AST_VECTOR_SIZE(matches); x++) {<br>+ len += strlen(AST_VECTOR_GET(matches, x));<br> }<br>- ast_free(matches);<br>+<br>+ /* the matches plus NULL terminator. */<br>+ buf = ast_malloc(len + 1);<br>+ if (!buf) {<br>+ AST_VECTOR_CALLBACK_VOID(matches, ast_free);<br>+ AST_VECTOR_PTR_FREE(matches);<br>+<br>+ return CLI_FAILURE;<br>+ }<br>+<br>+ len = 0;<br>+ for (x = 0; x < AST_VECTOR_SIZE(matches); x++) {<br>+ len += sprintf(buf + len, "%s ", AST_VECTOR_GET(matches, x));<br>+ }<br>+<br>+ AST_VECTOR_CALLBACK_VOID(matches, ast_free);<br>+ AST_VECTOR_PTR_FREE(matches);<br> }<br> <br>- if (buf) {<br>- ast_cli(a->fd, "%s%s",buf, AST_CLI_COMPLETE_EOF);<br>- ast_free(buf);<br>- } else<br>- ast_cli(a->fd, "NULL\n");<br>+ ast_cli(a->fd, "%s%s", buf ? buf : "", AST_CLI_COMPLETE_EOF);<br>+ ast_free(buf);<br> <br> return CLI_SUCCESS;<br> }<br>@@ -2500,91 +2500,164 @@<br> /*! \brief Return the number of unique matches for the generator */<br> int ast_cli_generatornummatches(const char *text, const char *word)<br> {<br>- int matches = 0, i = 0;<br>- char *buf = NULL, *oldbuf = NULL;<br>+ int matches = 0;<br>+ struct ast_vector_string *results;<br> <br>- while ((buf = ast_cli_generator(text, word, i++))) {<br>- if (!oldbuf || strcmp(buf,oldbuf))<br>- matches++;<br>- if (oldbuf)<br>- ast_free(oldbuf);<br>- oldbuf = buf;<br>+ results = ast_cli_completion_vector(text, word);<br>+ if (results) {<br>+ matches = AST_VECTOR_SIZE(results);<br>+<br>+ AST_VECTOR_CALLBACK_VOID(results, ast_free);<br>+ AST_VECTOR_PTR_FREE(results);<br> }<br>- if (oldbuf)<br>- ast_free(oldbuf);<br>+<br> return matches;<br>-}<br>-<br>-static void destroy_match_list(char **match_list, int matches)<br>-{<br>- if (match_list) {<br>- int idx;<br>-<br>- for (idx = 1; idx < matches; ++idx) {<br>- ast_free(match_list[idx]);<br>- }<br>- ast_free(match_list);<br>- }<br> }<br> <br> char **ast_cli_completion_matches(const char *text, const char *word)<br> {<br>- char **match_list = NULL, *retstr, *prevstr;<br>- char **new_list;<br>- size_t match_list_len, max_equal, which, i;<br>- int matches = 0;<br>+ struct ast_vector_string *results = ast_cli_completion_vector(text, word);<br>+ char **ret = NULL;<br> <br>- /* leave entry 0 free for the longest common substring */<br>- match_list_len = 1;<br>- while ((retstr = ast_cli_generator(text, word, matches)) != NULL) {<br>- if (matches + 1 >= match_list_len) {<br>- match_list_len <<= 1;<br>- new_list = ast_realloc(match_list, match_list_len * sizeof(*match_list));<br>- if (!new_list) {<br>- destroy_match_list(match_list, matches);<br>- return NULL;<br>- }<br>- match_list = new_list;<br>+ if (!results) {<br>+ return NULL;<br>+ }<br>+<br>+ if (AST_VECTOR_APPEND(results, NULL)) {<br>+ ret = AST_VECTOR_STEAL_ELEMENTS(results);<br>+ }<br>+<br>+ AST_VECTOR_CALLBACK_VOID(results, ast_free);<br>+ AST_VECTOR_PTR_FREE(results);<br>+<br>+ return ret;<br>+}<br>+<br>+AST_THREADSTORAGE_RAW(completion_storage);<br>+<br>+void ast_cli_completion_vector_save(struct ast_vector_string *matches)<br>+{<br>+ struct ast_vector_string *old_matches = ast_threadstorage_get_ptr(&completion_storage);<br>+<br>+ ast_threadstorage_set_ptr(&completion_storage, matches);<br>+<br>+ if (old_matches) {<br>+ /* If this happens it is a bug in a CLI generator. */<br>+ ast_log(LOG_ERROR, "ast_cli_completion_vector_save purging old results.\n");<br>+ ast_assert(0);<br>+<br>+ AST_VECTOR_CALLBACK_VOID(old_matches, ast_free);<br>+ AST_VECTOR_PTR_FREE(old_matches);<br>+ }<br>+}<br>+<br>+static int ast_el_sort_compare(const void *i1, const void *i2)<br>+{<br>+ char *s1 = ((char **)i1)[0];<br>+ char *s2 = ((char **)i2)[0];<br>+<br>+ return strcasecmp(s1, s2);<br>+}<br>+<br>+struct ast_vector_string *ast_cli_completion_vector(const char *text, const char *word)<br>+{<br>+ struct ast_vector_string *result;<br>+ struct ast_vector_string *saved;<br>+ char *retstr;<br>+ char *prevstr;<br>+ size_t max_equal;<br>+ size_t which = 0;<br>+ size_t i;<br>+<br>+ result = ast_calloc(1, sizeof(*result));<br>+ if (!result) {<br>+ return NULL;<br>+ }<br>+<br>+ while ((retstr = ast_cli_generator(text, word, which)) != NULL) {<br>+ if (AST_VECTOR_APPEND(result, retstr)) {<br>+ ast_free(retstr);<br>+<br>+ goto clean_result;<br> }<br>- match_list[++matches] = retstr;<br>+<br>+ ++which;<br> }<br> <br>- if (!match_list) {<br>- return match_list; /* NULL */<br>+ saved = ast_threadstorage_get_ptr(&completion_storage);<br>+ if (saved) {<br>+ ast_threadstorage_set_ptr(&completion_storage, NULL);<br>+ if (!AST_VECTOR_SIZE(saved)) {<br>+ /* Ignore zero length lists. */<br>+ AST_VECTOR_PTR_FREE(saved);<br>+ saved = NULL;<br>+ }<br> }<br>+<br>+ if (!AST_VECTOR_SIZE(result)) {<br>+ if (!saved) {<br>+ AST_VECTOR_PTR_FREE(result);<br>+<br>+ return NULL;<br>+ }<br>+<br>+ AST_VECTOR_PTR_FREE(result);<br>+ result = saved;<br>+ } else if (saved) {<br>+ ast_assert(0);<br>+ ast_log(LOG_ERROR, "CLI completion functions must return results or call "<br>+ "ast_cli_completion_vector_save, not both.\n");<br>+ AST_VECTOR_CALLBACK_VOID(saved, ast_free);<br>+ AST_VECTOR_PTR_FREE(saved);<br>+ ast_threadstorage_set_ptr(&completion_storage, NULL);<br>+<br>+ goto clean_result;<br>+ }<br>+<br>+ /* Sort now so we can remove duplicates. */<br>+ AST_VECTOR_QSORT(result, ast_el_sort_compare);<br>+<br>+ prevstr = AST_VECTOR_GET(result, 0);<br>+ max_equal = strlen(prevstr);<br>+ which = 1;<br> <br> /* Find the longest substring that is common to all results<br> * (it is a candidate for completion), and store a copy in entry 0.<br> */<br>- prevstr = match_list[1];<br>- max_equal = strlen(prevstr);<br>- for (which = 2; which <= matches; which++) {<br>- for (i = 0; i < max_equal && toupper(prevstr[i]) == toupper(match_list[which][i]); i++)<br>+ while (which < AST_VECTOR_SIZE(result)) {<br>+ char *str = AST_VECTOR_GET(result, which);<br>+<br>+ /* Check for and remove duplicate strings. */<br>+ if (!strcasecmp(prevstr, str)) {<br>+ AST_VECTOR_REMOVE(result, which, 1);<br>+ ast_free(str);<br>+<br> continue;<br>- max_equal = i;<br>- }<br>-<br>- retstr = ast_malloc(max_equal + 1);<br>- if (!retstr) {<br>- destroy_match_list(match_list, matches);<br>- return NULL;<br>- }<br>- ast_copy_string(retstr, match_list[1], max_equal + 1);<br>- match_list[0] = retstr;<br>-<br>- /* ensure that the array is NULL terminated */<br>- if (matches + 1 >= match_list_len) {<br>- new_list = ast_realloc(match_list, (match_list_len + 1) * sizeof(*match_list));<br>- if (!new_list) {<br>- ast_free(retstr);<br>- destroy_match_list(match_list, matches);<br>- return NULL;<br> }<br>- match_list = new_list;<br>- }<br>- match_list[matches + 1] = NULL;<br> <br>- return match_list;<br>+ for (i = 0; i < max_equal && toupper(prevstr[i]) == toupper(str[i]); i++) {<br>+ continue;<br>+ }<br>+<br>+ max_equal = i;<br>+ prevstr = str;<br>+ ++which;<br>+ }<br>+<br>+ /* Insert longest match to position 0. */<br>+ retstr = ast_strndup(AST_VECTOR_GET(result, 0), max_equal);<br>+ if (!retstr || AST_VECTOR_INSERT_AT(result, 0, retstr)) {<br>+ ast_free(retstr);<br>+ goto clean_result;<br>+ }<br>+<br>+ return result;<br>+<br>+clean_result:<br>+ AST_VECTOR_CALLBACK_VOID(result, ast_free);<br>+ AST_VECTOR_PTR_FREE(result);<br>+<br>+ return NULL;<br> }<br> <br> /*! \brief returns true if there are more words to match */<br>diff --git a/main/loader.c b/main/loader.c<br>index add6a42..107f87b 100644<br>--- a/main/loader.c<br>+++ b/main/loader.c<br>@@ -727,17 +727,16 @@<br> }<br> }<br> <br>-static char *module_load_helper(const char *word, int state)<br>+static int module_load_helper(const char *word, struct ast_vector_string *result)<br> {<br> struct ast_module *mod;<br>- int which = 0;<br> char *name;<br>- char *ret = NULL;<br>+ int ret = 0;<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 ast_vector_string running_modules;<br> <br> AST_VECTOR_INIT(&running_modules, 200);<br> <br>@@ -766,8 +765,12 @@<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>+ if (!AST_VECTOR_GET_CMP(&running_modules, name, !strcasecmp)) {<br>+ name = ast_strdup(name);<br>+ if (!name || AST_VECTOR_APPEND(result, name)) {<br>+ ast_free(name);<br>+ ret = -1;<br>+ }<br> }<br> <br> ast_std_free(editline_ret);<br>@@ -783,42 +786,63 @@<br> char *ast_module_helper(const char *line, const char *word, int pos, int state, int rpos, enum ast_module_helper_type type)<br> {<br> struct ast_module *mod;<br>- int which = 0;<br> int wordlen = strlen(word);<br>- char *ret = NULL;<br>+ int ret = 0;<br>+ char *name;<br>+ struct ast_vector_string *result = ast_calloc(1, sizeof(*result));<br> <br>- if (pos != rpos) {<br>+ if (pos != rpos || !result) {<br>+ ast_free(result);<br>+<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>+ if (!strncasecmp(word, reload_classes[idx].name, wordlen)) {<br>+ name = ast_strdup(reload_classes[idx].name);<br>+ if (!name || AST_VECTOR_APPEND(result, name)) {<br>+ ast_free(name);<br>+ ret = -1;<br>+<br>+ break;<br>+ }<br> }<br> }<br> }<br> <br>- AST_DLLIST_LOCK(&module_list);<br>- AST_DLLIST_TRAVERSE(&module_list, mod, entry) {<br>- if (!module_matches_helper_type(mod, type)) {<br>- continue;<br>- }<br>+ if (type == AST_MODULE_HELPER_LOAD) {<br>+ ret = module_load_helper(word, result);<br>+ } else if (!ret) {<br>+ AST_DLLIST_LOCK(&module_list);<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>+ if (!strncasecmp(word, mod->resource, wordlen)) {<br>+ name = ast_strdup(mod->resource);<br>+ if (!name || AST_VECTOR_APPEND(result, name)) {<br>+ ast_free(name);<br>+<br>+ ret = -1;<br>+ break;<br>+ }<br>+ }<br> }<br>+ AST_DLLIST_UNLOCK(&module_list);<br> }<br>- AST_DLLIST_UNLOCK(&module_list);<br> <br>- return ret;<br>+ if (!ret && AST_VECTOR_SIZE(result)) {<br>+ ast_cli_completion_vector_save(result);<br>+ } else {<br>+ AST_VECTOR_CALLBACK_VOID(result, ast_free);<br>+ AST_VECTOR_PTR_FREE(result);<br>+ }<br>+<br>+ return NULL;<br> }<br> <br> void ast_process_pending_reloads(void)<br>diff --git a/tests/test_substitution.c b/tests/test_substitution.c<br>index 3a1dc1f..2dad076 100644<br>--- a/tests/test_substitution.c<br>+++ b/tests/test_substitution.c<br>@@ -225,6 +225,7 @@<br> struct ast_channel *c;<br> int i;<br> enum ast_test_result_state res = AST_TEST_PASS;<br>+ struct ast_vector_string *funcs;<br> <br> switch (cmd) {<br> case TEST_INIT:<br>@@ -302,11 +303,12 @@<br> #undef TEST<br> <br> /* For testing dialplan functions */<br>- for (i = 0; ; i++) {<br>- char *cmd = ast_cli_generator("core show function", "", i);<br>- if (cmd == NULL) {<br>- break;<br>- }<br>+ funcs = ast_cli_completion_vector("core show function", "");<br>+<br>+ /* Skip "best match" element 0 */<br>+ for (i = 1; funcs && i < AST_VECTOR_SIZE(funcs); i++) {<br>+ char *cmd = AST_VECTOR_GET(funcs, i);<br>+<br> if (strcmp(cmd, "CHANNEL") && strcmp(cmd, "CALLERID") && strncmp(cmd, "CURL", 4) &&<br> strncmp(cmd, "AES", 3) && strncmp(cmd, "BASE64", 6) &&<br> strcmp(cmd, "CDR") && strcmp(cmd, "ENV") && strcmp(cmd, "GLOBAL") &&<br>@@ -324,7 +326,10 @@<br> ast_free(cmd);<br> }<br> <br>+ AST_VECTOR_CALLBACK_VOID(funcs, ast_free);<br>+ AST_VECTOR_PTR_FREE(funcs);<br> ast_hangup(c);<br>+<br> return res;<br> }<br> <br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/6996">change 6996</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/6996"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>
<div style="display:none"> Gerrit-Change-Id: Ie527d3d5e8efcf5eaddf882db8b1f904e86543a8 </div>
<div style="display:none"> Gerrit-Change-Number: 6996 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Corey Farrell <git@cfware.com> </div>