[Asterisk-code-review] cli: Various fixes and improvements for CLI handling (asterisk[13])
Sean Bright
asteriskteam at digium.com
Sun Feb 26 13:40:22 CST 2017
Sean Bright has uploaded a new change for review. ( https://gerrit.asterisk.org/5087 )
Change subject: cli: Various fixes and improvements for CLI handling
......................................................................
cli: Various fixes and improvements for CLI handling
This is a general cleanup of CLI and completion handling. Some of the
relevant highlights:
* Arguments that contain spaces can now be completed properly, whether
they are quoted (e.g. 'queue show "<TAB>...') or backslash escaped
(e.g. 'queue show Customer\ Support\ Que<TAB>').
* Because of the way completions are handled with the remote console,
any completion option with spaces in it was treated as multiple
completion options. This has been corrected.
* The local and remote console completion code calculated the number
of matches differently, resulting in inconsistent sorting of options.
* The remote console would lock up if given a long input string to
complete.
* Completion candidates were de-duplicated in multiple places, we now
do it as early as possible before handing off to calling code.
* If we are only going to show a single line of completion options to
the user, we no longer columnize the output.
Change-Id: Id911e1035525b33fc467e365a2cd6259aed1e90e
---
M include/asterisk/cli.h
M main/asterisk.c
M main/cli.c
3 files changed, 311 insertions(+), 109 deletions(-)
git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/87/5087/1
diff --git a/include/asterisk/cli.h b/include/asterisk/cli.h
index c51d89e..15fa9f3 100644
--- a/include/asterisk/cli.h
+++ b/include/asterisk/cli.h
@@ -322,6 +322,32 @@
*/
int ast_cli_allow_at_shutdown(struct ast_cli_entry *e);
+/*!
+ * \brief Escape spaces and backslashes in CLI completions
+ *
+ * Escapes spaces and backslashes in \c str with a backslash. If no
+ * escaping is done, this function returns a NULL pointer. If escaping
+ * does occur, the returned pointer is non-NULL and must be freed by
+ * the caller.
+ *
+ * \returns NULL if no escaping was done, a dynamically allocated
+ * string otherwise.
+ */
+char *ast_cli_completion_option_escape(const char *str);
+
+/*!
+ * \brief Unescape spaces and backslashes in CLI completions
+ *
+ * Unescapes backslash escaped spaces and backslashes in \c str. If no
+ * unescaping is done, this function returns a NULL pointer. If
+ * unescaping does occur, the returned pointer is non-NULL and must be
+ * freed by the caller.
+ *
+ * \returns NULL if no unescaping was done, a dynamically allocated
+ * string otherwise.
+ */
+char *ast_cli_completion_option_unescape(const char *str);
+
#if defined(__cplusplus) || defined(c_plusplus)
}
#endif
diff --git a/main/asterisk.c b/main/asterisk.c
index 076ae6d..bbc9ede 100644
--- a/main/asterisk.c
+++ b/main/asterisk.c
@@ -3081,69 +3081,92 @@
}
}
-static char **ast_el_strtoarr(char *buf)
+static char *strsep_with_escapes(char **stringp, const char *delim)
+{
+ char *match;
+ char *save = *stringp;
+ char *front = save;
+
+ if (!stringp || !*stringp) {
+ return NULL;
+ }
+
+ while ((match = strpbrk(front, delim))) {
+ /* We have a match, was it escaped? */
+ if (match == *stringp || *(match - 1) != '\\') {
+ *match = 0;
+ *stringp = match + 1;
+ return save;
+ }
+ front = match + 1;
+ }
+
+ *stringp = NULL;
+ return save;
+}
+
+static char **delimited_string_to_array(char *buf, size_t *count)
{
char *retstr;
char **match_list = NULL;
char **new_list;
- size_t match_list_len = 1;
- int matches = 0;
+ size_t match_list_capacity = 1;
- while ((retstr = strsep(&buf, " "))) {
+ *count = 0;
+
+ while ((retstr = strsep_with_escapes(&buf, " "))) {
+ char *unescaped;
+
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 (*count + 1 >= match_list_capacity) {
+ match_list_capacity <<= 1;
+ new_list = ast_realloc(match_list, match_list_capacity * sizeof(char *));
if (!new_list) {
- destroy_match_list(match_list, matches);
+ destroy_match_list(match_list, *count);
return NULL;
}
match_list = new_list;
}
- retstr = ast_strdup(retstr);
+ unescaped = ast_cli_completion_option_unescape(retstr);
+ if (unescaped) {
+ retstr = unescaped;
+ } else {
+ retstr = ast_strdup(retstr);
+ }
+
if (!retstr) {
- destroy_match_list(match_list, matches);
+ destroy_match_list(match_list, *count);
return NULL;
}
- match_list[matches++] = retstr;
+ match_list[(*count)++] = retstr;
}
if (!match_list) {
return NULL;
}
- if (matches >= match_list_len) {
- new_list = ast_realloc(match_list, (match_list_len + 1) * sizeof(char *));
+ if (*count >= match_list_capacity) {
+ new_list = ast_realloc(match_list, (match_list_capacity + 1) * sizeof(char *));
if (!new_list) {
- destroy_match_list(match_list, matches);
+ destroy_match_list(match_list, *count);
return NULL;
}
match_list = new_list;
}
- match_list[matches] = NULL;
+ match_list[*count] = NULL;
return match_list;
-}
-
-static int ast_el_sort_compare(const void *i1, const void *i2)
-{
- 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;
+ size_t screenwidth = 0;
+ size_t numoutput = 0, numoutputline = 0;
screenwidth = ast_get_termcols(STDOUT_FILENO);
@@ -3159,49 +3182,66 @@
idx = 1;
- qsort(&matches[0], (size_t)(len), sizeof(char *), ast_el_sort_compare);
-
- for (; count > 0; count--) {
- numoutputline = 0;
+ /* If we only have one line, don't columnize output */
+ if (count == 1) {
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++;
- numoutputline++;
- fprintf(stdout, "%-*s ", max, matches[idx]);
+ fprintf(stdout, "%s%s", i == 0 ? "" : " ", matches[idx]);
ast_free(matches[idx]);
matches[idx] = NULL;
}
- if (numoutputline > 0)
- fprintf(stdout, "\n");
+ fprintf(stdout, "\n");
+ } else {
+ for (; count > 0; count--) {
+ for (i = 0; i < limit && matches[idx]; i++, idx++) {
+ numoutput++;
+ numoutputline++;
+ fprintf(stdout, "%s%-*s", i == 0 ? "" : " ", max, matches[idx]);
+ ast_free(matches[idx]);
+ matches[idx] = NULL;
+ }
+ if (numoutputline > 0)
+ fprintf(stdout, "\n");
+ }
}
return numoutput;
}
+static char *escape_quotes(const char *str)
+{
+ const char *cur;
+ char *escaped, *dest;
+
+ escaped = ast_calloc(strlen(str) * 2 + 1, sizeof(char));
+ if (!escaped) {
+ return NULL;
+ }
+
+ for (cur = str, dest = escaped; *cur; cur++) {
+ if (*cur == '"')
+ *dest++ = '\\';
+ *dest++ = *cur;
+ }
+
+ return escaped;
+}
static char *cli_complete(EditLine *editline, int ch)
{
- int len = 0;
- char *ptr;
- int nummatches = 0;
- char **matches;
int retval = CC_ERROR;
- char buf[2048], savechr;
- int res;
+ size_t nummatches = 0;
+ char *ptr, savechr;
+ char **matches;
+ char **cur;
+ int is_quoted = 0;
- LineInfo *lf = (LineInfo *)el_line(editline);
+ LineInfo *lf = (LineInfo *) el_line(editline);
- savechr = *(char *)lf->cursor;
- *(char *)lf->cursor = '\0';
- ptr = (char *)lf->cursor;
+ /* XXX: These next few lines look awful */
+ savechr = *(char *) lf->cursor;
+ *(char *) lf->cursor = '\0';
+ ptr = (char *) lf->cursor;
if (ptr) {
while (ptr > lf->buffer) {
if (isspace(*ptr)) {
@@ -3210,82 +3250,100 @@
}
ptr--;
}
+
+ if (*ptr == '"') {
+ ptr++;
+ is_quoted = 1;
+ }
}
- len = lf->cursor - ptr;
-
if (ast_opt_remote) {
- snprintf(buf, sizeof(buf), "_COMMAND NUMMATCHES \"%s\" \"%s\"", lf->buffer, ptr);
- fdsend(ast_consock, buf);
- if ((res = read(ast_consock, buf, sizeof(buf) - 1)) < 0) {
- return (char*)(CC_ERROR);
+ char *match_buf;
+ size_t match_buf_capacity = 2048;
+ int res = 0, match_buf_used = 0;
+ char cmd_buf[2048];
+ char *escaped_buffer, *escaped_ptr;
+
+ /* Start with a 2048 byte buffer */
+ if (!(match_buf = ast_malloc(match_buf_capacity))) {
+ *((char *) lf->cursor) = savechr;
+ return (char *)(CC_ERROR);
}
- buf[res] = '\0';
- nummatches = atoi(buf);
- if (nummatches > 0) {
- char *mbuf;
- char *new_mbuf;
- int mlen = 0, maxmbuf = 2048;
+ escaped_buffer = escape_quotes(lf->buffer);
+ escaped_ptr = escape_quotes(ptr);
- /* 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;
+ if (!escaped_buffer || !escaped_ptr) {
+ *((char *) lf->cursor) = savechr;
+ return (char *)(CC_ERROR);
+ }
+
+ res = snprintf(cmd_buf, sizeof(cmd_buf), "_COMMAND MATCHESARRAY \"%s\" \"%s\"", escaped_buffer, escaped_ptr);
+ if (res >= sizeof(cmd_buf)) {
+ ast_free(match_buf);
+ *((char *) lf->cursor) = savechr;
+ return (char *)(CC_ERROR);
+ }
+
+ ast_free(escaped_buffer);
+ ast_free(escaped_ptr);
+
+ fdsend(ast_consock, cmd_buf);
+ match_buf[0] = '\0';
+ res = 0;
+ while (!strstr(match_buf, AST_CLI_COMPLETE_EOF) && res != -1) {
+ if (match_buf_used + 1024 > match_buf_capacity) {
+ char *resized;
+
+ /* Every step increment buffer 1024 bytes */
+ match_buf_capacity += 1024;
+ resized = ast_realloc(match_buf, match_buf_capacity);
+ if (!resized) {
+ ast_free(match_buf);
+ *((char *) lf->cursor) = savechr;
+ return (char *)(CC_ERROR);
}
- /* Only read 1024 bytes at a time */
- res = read(ast_consock, mbuf + mlen, 1024);
- if (res > 0)
- mlen += res;
+ match_buf = resized;
}
- mbuf[mlen] = '\0';
+ /* Only read 1024 bytes at a time */
+ res = read(ast_consock, match_buf + match_buf_used, 1024);
+ if (res > 0)
+ match_buf_used += res;
+ }
+ match_buf[match_buf_used] = '\0';
- matches = ast_el_strtoarr(mbuf);
- ast_free(mbuf);
- } else
- matches = (char **) NULL;
+ matches = delimited_string_to_array(match_buf, &nummatches);
+ ast_free(match_buf);
} 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;
+ matches = ast_cli_completion_matches((char *) lf->buffer, ptr);
+ for (cur = matches; cur && *cur; cur++) {
+ nummatches++;
}
}
if (matches) {
int i;
- int matches_num, maxlen, match_len;
if (matches[0][0] != '\0') {
- el_deletestr(editline, (int) len);
+ if (!is_quoted) {
+ char *escaped = ast_cli_completion_option_escape(matches[0]);
+ if (escaped) {
+ ast_free(matches[0]);
+ matches[0] = escaped;
+ }
+ }
+ el_deletestr(editline, lf->cursor - ptr);
el_insertstr(editline, matches[0]);
retval = CC_REFRESH;
}
if (nummatches == 1) {
/* Found an exact match */
- el_insertstr(editline, " ");
+ el_insertstr(editline, is_quoted ? "\" " : " ");
retval = CC_REFRESH;
} else {
+ size_t matches_num, match_len, maxlen;
+
/* Must be more than one match */
for (i = 1, maxlen = 0; matches[i]; i++) {
match_len = strlen(matches[i]);
@@ -3293,12 +3351,12 @@
maxlen = match_len;
}
matches_num = i - 1;
- if (matches_num >1) {
+ if (matches_num > 1) {
fprintf(stdout, "\n");
ast_cli_display_match_list(matches, nummatches, maxlen);
retval = CC_REDISPLAY;
} else {
- el_insertstr(editline," ");
+ el_insertstr(editline, is_quoted ? "\" " : " ");
retval = CC_REFRESH;
}
}
diff --git a/main/cli.c b/main/cli.c
index 1af9177..9de51ac 100644
--- a/main/cli.c
+++ b/main/cli.c
@@ -1329,6 +1329,12 @@
matches = ast_cli_completion_matches(a->argv[2], a->argv[3]);
if (matches) {
for (x=0; matches[x]; x++) {
+ char *escaped = ast_cli_completion_option_escape(matches[x]);
+ if (escaped) {
+ ast_free(matches[x]);
+ matches[x] = escaped;
+ }
+
matchlen = strlen(matches[x]) + 1;
if (len + matchlen >= buflen) {
buflen += matchlen * 3;
@@ -1690,6 +1696,72 @@
ast_channel_unref(chan);
return CLI_SUCCESS;
+}
+
+char *ast_cli_completion_option_escape(const char *opt)
+{
+ const char *cur = opt;
+ char *escaped = NULL, *dest;
+ size_t opt_len = strlen(opt);
+
+ while (*cur) {
+ if (*cur == ' ' || *cur == '\\') {
+ if (!escaped) {
+ dest = escaped = ast_calloc(opt_len * 2 + 1, sizeof(char));
+ if (!escaped) {
+ /* Can't escape, so we bail out */
+ return NULL;
+ }
+ memcpy(dest, opt, cur - opt);
+ dest += cur - opt;
+ }
+ *dest++ = '\\';
+ *dest++ = *cur;
+ } else if (escaped) {
+ *dest++ = *cur;
+ }
+
+ cur++;
+ }
+
+ return escaped;
+}
+
+char *ast_cli_completion_option_unescape(const char *opt)
+{
+ const char *cur = opt;
+ char *unescaped = NULL, *dest;
+ size_t opt_len = strlen(opt);
+
+ while (*cur) {
+ if (*cur == '\\') {
+ char next = *(cur + 1);
+ if (!unescaped) {
+ dest = unescaped = ast_calloc(opt_len + 1, sizeof(char));
+ if (!unescaped) {
+ /* Can't unescape, so we bail out */
+ return NULL;
+ }
+ memcpy(dest, opt, cur - opt);
+ dest += cur - opt;
+ }
+
+ if (next == ' ' || next == '\\') {
+ *dest++ = next;
+ cur++;
+ } else {
+ ast_log(LOG_ERROR, "Invalid escape sequence found in \"%s\"\n", opt);
+ ast_free(unescaped);
+ return NULL;
+ }
+ } else if (unescaped) {
+ *dest++ = *cur;
+ }
+
+ cur++;
+ }
+
+ return unescaped;
}
/*
@@ -2535,16 +2607,36 @@
}
}
+static int sort_completions(const void *i1, const void *i2)
+{
+ char *s1, *s2;
+
+ s1 = ((char **) i1)[0];
+ s2 = ((char **) i2)[0];
+
+ return strcasecmp(s1, s2);
+}
+
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;
+ size_t matches = 0;
+ int state = 0;
/* leave entry 0 free for the longest common substring */
match_list_len = 1;
- while ((retstr = ast_cli_generator(text, word, matches)) != NULL) {
+ while ((retstr = ast_cli_generator(text, word, state)) != NULL) {
+ if (matches > 0) {
+ /* Do an initial deduplication */
+ if (!strcmp(retstr, match_list[matches])) {
+ ast_free(retstr);
+ state++;
+ continue;
+ }
+ }
+
if (matches + 1 >= match_list_len) {
match_list_len <<= 1;
new_list = ast_realloc(match_list, match_list_len * sizeof(*match_list));
@@ -2555,12 +2647,38 @@
match_list = new_list;
}
match_list[++matches] = retstr;
+ state++;
}
if (!match_list) {
return match_list; /* NULL */
}
+ /* Sort */
+ qsort(&match_list[1], matches, sizeof(char *), sort_completions);
+
+ if (matches > 1) {
+ /*
+ * The internal CLI completion handling will have already given us
+ * back a sorted list which we deduplicated earlier, but the custom
+ * completion handlers were under no obligation to do so, so we have
+ * to do another round after we've sorted everything.
+ */
+ prevstr = match_list[1];
+ for (i = 2; i <= matches;) {
+ if (!strcmp(prevstr, match_list[i])) {
+ ast_free(match_list[i]);
+ if (matches - i)
+ memmove(&match_list[i],
+ &match_list[i + 1],
+ (matches - i) * sizeof(char *));
+ matches--;
+ } else {
+ prevstr = match_list[i++];
+ }
+ }
+ }
+
/* Find the longest substring that is common to all results
* (it is a candidate for completion), and store a copy in entry 0.
*/
--
To view, visit https://gerrit.asterisk.org/5087
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id911e1035525b33fc467e365a2cd6259aed1e90e
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Sean Bright <sean.bright at gmail.com>
More information about the asterisk-code-review
mailing list