[Asterisk-code-review] CLI: Create ast cli completion vector. (asterisk[13])

[Joshua Colp]

Joshua Colp has submitted this change and it was merged. ( https://gerrit.asterisk.org/7257 )

Change subject: CLI: Create ast_cli_completion_vector.
......................................................................

CLI: Create ast_cli_completion_vector.

This is a rewrite of ast_cli_completion_matches using a vector to build
the list.  The original function calls the vector version, NULL
terminates the vector and extracts the elements array.

One change in behavior the results are now sorted and deduplicated. This
will solve bugs where some duplicate checking was done before the list
was sorted.

Change-Id: Iede20c5b4d965fa5ec71fda136ce9425eeb69519
---
M include/asterisk/cli.h
M main/cli.c
2 files changed, 99 insertions(+), 56 deletions(-)

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



diff --git a/include/asterisk/cli.h b/include/asterisk/cli.h
index c51d89e..7bf473c 100644
--- a/include/asterisk/cli.h
+++ b/include/asterisk/cli.h
@@ -301,6 +301,27 @@
 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 Command completion for the list of active channels.
  *
  * This can be called from a CLI command completion function that wants to
diff --git a/main/cli.c b/main/cli.c
index 64882a3..a9f02fd 100644
--- a/main/cli.c
+++ b/main/cli.c
@@ -2476,76 +2476,98 @@
 	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 *vec = ast_cli_completion_vector(text, word);
+	char **match_list;
 
-	/* 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 (!vec) {
+		return NULL;
+	}
+
+	if (AST_VECTOR_APPEND(vec, NULL)) {
+		/* We failed to NULL terminate the elements */
+		AST_VECTOR_CALLBACK_VOID(vec, ast_free);
+		AST_VECTOR_PTR_FREE(vec);
+
+		return NULL;
+	}
+
+	match_list = AST_VECTOR_STEAL_ELEMENTS(vec);
+	AST_VECTOR_PTR_FREE(vec);
+
+	return match_list;
+}
+
+struct ast_vector_string *ast_cli_completion_vector(const char *text, const char *word)
+{
+	char *retstr, *prevstr;
+	size_t max_equal;
+	size_t which = 0;
+	struct ast_vector_string *vec = ast_calloc(1, sizeof(*vec));
+
+	if (!vec) {
+		return NULL;
+	}
+
+	while ((retstr = ast_cli_generator(text, word, which)) != NULL) {
+		if (AST_VECTOR_ADD_SORTED(vec, retstr, strcasecmp)) {
+			ast_free(retstr);
+
+			goto vector_cleanup;
 		}
-		match_list[++matches] = retstr;
+
+		++which;
 	}
 
-	if (!match_list) {
-		return match_list; /* NULL */
+	if (!AST_VECTOR_SIZE(vec)) {
+		AST_VECTOR_PTR_FREE(vec);
+
+		return NULL;
 	}
+
+	prevstr = AST_VECTOR_GET(vec, 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++)
-			continue;
-		max_equal = i;
-	}
+	while (which < AST_VECTOR_SIZE(vec)) {
+		size_t i = 0;
 
-	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) {
+		retstr = AST_VECTOR_GET(vec, which);
+		/* Check for and remove duplicate strings. */
+		if (!strcasecmp(prevstr, retstr)) {
+			AST_VECTOR_REMOVE(vec, which, 1);
 			ast_free(retstr);
-			destroy_match_list(match_list, matches);
-			return NULL;
-		}
-		match_list = new_list;
-	}
-	match_list[matches + 1] = NULL;
 
-	return match_list;
+			continue;
+		}
+
+		while (i < max_equal && toupper(prevstr[i]) == toupper(retstr[i])) {
+			i++;
+		}
+
+		max_equal = i;
+		prevstr = retstr;
+		++which;
+	}
+
+	/* Insert longest match to position 0. */
+	retstr = ast_strndup(AST_VECTOR_GET(vec, 0), max_equal);
+	if (!retstr || AST_VECTOR_INSERT_AT(vec, 0, retstr)) {
+		ast_free(retstr);
+		goto vector_cleanup;
+	}
+
+	return vec;
+
+vector_cleanup:
+	AST_VECTOR_CALLBACK_VOID(vec, ast_free);
+	AST_VECTOR_PTR_FREE(vec);
+
+	return NULL;
 }
 
 /*! \brief returns true if there are more words to match */

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

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-MessageType: merged
Gerrit-Change-Id: Iede20c5b4d965fa5ec71fda136ce9425eeb69519
Gerrit-Change-Number: 7257
Gerrit-PatchSet: 1
Gerrit-Owner: Corey Farrell <git at cfware.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/20171121/65b451eb/attachment-0001.html>