[Asterisk-code-review] CLI: Refactor cli complete. (asterisk[master])

George Joseph asteriskteam at digium.com
Mon Nov 27 14:39:07 CST 2017


George Joseph has submitted this change and it was merged. ( https://gerrit.asterisk.org/7268 )

Change subject: CLI: Refactor cli_complete.
......................................................................

CLI: Refactor cli_complete.

* Stop using "_COMMAND NUMMATCHES" on remote consoles.  Using this
  command had doubled the amount of work needed from the Asterisk
  daemon for each completion request.
* Fix code formatting.
* Remove static buffer used to send the command, use the same buffer
  that will receive the results.
* Move sort from ast_cli_display_match_list.

Change-Id: Ie2211b519a3d4bec45bf46e0095bdd01d384cb69
---
M main/asterisk.c
1 file changed, 76 insertions(+), 66 deletions(-)

Approvals:
  Kevin Harwell: Looks good to me, but someone else must approve
  Joshua Colp: Looks good to me, but someone else must approve
  George Joseph: Looks good to me, approved; Approved for Submit



diff --git a/main/asterisk.c b/main/asterisk.c
index 46dc9a7..ec8ead1 100644
--- a/main/asterisk.c
+++ b/main/asterisk.c
@@ -3054,7 +3054,7 @@
 	return strcasecmp(s1, s2);
 }
 
-static void ast_cli_display_match_list(char **matches, int len, int max)
+static void ast_cli_display_match_list(char **matches, int max)
 {
 	int idx = 1;
 	/* find out how many entries can be put on one line, with two spaces between strings */
@@ -3063,8 +3063,6 @@
 	if (limit == 0) {
 		limit = 1;
 	}
-
-	qsort(&matches[0], (size_t)(len), sizeof(char *), ast_el_sort_compare);
 
 	for (;;) {
 		int numoutputline;
@@ -3095,7 +3093,7 @@
 	int nummatches = 0;
 	char **matches;
 	int retval = CC_ERROR;
-	char buf[2048], savechr;
+	char savechr;
 	int res;
 
 	LineInfo *lf = (LineInfo *)el_line(editline);
@@ -3116,65 +3114,80 @@
 	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);
-		}
-		buf[res] = '\0';
-		nummatches = atoi(buf);
+#define CMD_MATCHESARRAY "_COMMAND MATCHESARRAY \"%s\" \"%s\""
+		char *mbuf;
+		char *new_mbuf;
+		int mlen = 0, maxmbuf = 2048;
 
-		if (nummatches > 0) {
-			char *mbuf;
-			char *new_mbuf;
-			int mlen = 0, maxmbuf = 2048;
+		/* Start with a 2048 byte buffer */
+		mbuf = ast_malloc(maxmbuf);
 
-			/* 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;
-				}
-				/* 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);
+		/* This will run snprintf twice at most. */
+		while (mbuf && (mlen = snprintf(mbuf, maxmbuf, CMD_MATCHESARRAY, lf->buffer, ptr)) > maxmbuf) {
+			/* Return value does not include space for NULL terminator. */
+			maxmbuf = mlen + 1;
 			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;
+			mbuf = ast_malloc(maxmbuf);
 		}
+
+		if (!mbuf) {
+			*((char *) lf->cursor) = savechr;
+
+			return (char *)(CC_ERROR);
+		}
+
+		fdsend(ast_consock, mbuf);
+		res = 0;
+		mlen = 0;
+		mbuf[0] = '\0';
+
+		while (!strstr(mbuf, AST_CLI_COMPLETE_EOF) && res != -1) {
+			if (mlen + 1024 > maxmbuf) {
+				/* Expand buffer to the next 1024 byte increment. */
+				maxmbuf = mlen + 1024;
+				new_mbuf = ast_realloc(mbuf, maxmbuf);
+				if (!new_mbuf) {
+					ast_free(mbuf);
+					*((char *) lf->cursor) = savechr;
+
+					return (char *)(CC_ERROR);
+				}
+				mbuf = new_mbuf;
+			}
+			/* 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_matches((char *)lf->buffer,ptr);
 	}
 
 	if (matches) {
 		int i;
-		int matches_num, maxlen, match_len;
+		int maxlen, match_len;
+
+		while (matches[nummatches + 1]) {
+			nummatches++;
+		}
+
+		if (ast_opt_remote && nummatches > 1) {
+			qsort(&matches[0], (size_t)(nummatches), sizeof(char *), ast_el_sort_compare);
+			nummatches = 1;
+			i = 1;
+			while (matches[i + 1]) {
+				if (strcasecmp(matches[i], matches[i + 1])) {
+					/* don't count duplicates. */
+					nummatches++;
+				}
+				i++;
+			}
+		}
 
 		if (matches[0][0] != '\0') {
 			el_deletestr(editline, (int) len);
@@ -3190,21 +3203,18 @@
 			/* Must be more than one match */
 			for (i = 1, maxlen = 0; matches[i]; i++) {
 				match_len = strlen(matches[i]);
-				if (match_len > maxlen)
+				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++)
+		for (i = 0; matches[i]; i++) {
 			ast_free(matches[i]);
+		}
 		ast_free(matches);
 	}
 

-- 
To view, visit https://gerrit.asterisk.org/7268
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie2211b519a3d4bec45bf46e0095bdd01d384cb69
Gerrit-Change-Number: 7268
Gerrit-PatchSet: 3
Gerrit-Owner: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20171127/3992c173/attachment.html>


More information about the asterisk-code-review mailing list