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