Index: cli.c
===================================================================
--- cli.c	(revision 8259)
+++ cli.c	(working copy)
@@ -762,53 +745,48 @@
 	return RESULT_SUCCESS;
 }
 
-static char *complete_show_channels(const char *line, const char *word, int pos, int state)
+/*
+ * helper function to generate CLI matches from a fixed set of values.
+ * A NULL word is acceptable.
+ */
+char *ast_cli_complete(const char *word, char *const choices[], int state)
 {
-	static char *choices[] = { "concise", "verbose" };
-	int match = 0;
-	int x;
-	int wordlen;
+	int i, which = 0, len;
+	len = ast_strlen_zero(word) ? 0 : strlen(word);
 
-	if (pos != 2) 
-		return NULL;
-	
-	wordlen = strlen(word);
-
-	for (x = 0; x < sizeof(choices) / sizeof(choices[0]); x++) {
-		if (!strncasecmp(word, choices[x], wordlen)) {
-			match++;
-			if (match > state)
-				return strdup(choices[x]);
-		}
+	for (i = 0; choices[i]; i++) {
+		if ((!len || !strncasecmp(word, choices[i], len)) && ++which > state)
+			return ast_strdup(choices[i]);
 	}
-
 	return NULL;
 }
 
+static char *complete_show_channels(const char *line, const char *word, int pos, int state)
+{
+	static char *choices[] = { "concise", "verbose", NULL };
+
+	return (pos != 2) ? NULL : ast_cli_complete(word, choices, state);
+}
+
 static char *complete_ch_helper(const char *line, const char *word, int pos, int state, int rpos)
 {
 	struct ast_channel *c = NULL;
 	int which = 0;
 	int wordlen;
-	char *ret = NULL;
+	char notfound = '\0';
+	char *ret = &notfound; /* so NULL can break the loop */
 
 	if (pos != rpos)
 		return NULL;
 
 	wordlen = strlen(word);	
 
-	while ((c = ast_channel_walk_locked(c))) {
-		if (!strncasecmp(word, c->name, wordlen)) {
-			if (++which > state) {
-				ret = strdup(c->name);
-				ast_mutex_unlock(&c->lock);
-				break;
-			}
-		}
+	while (ret == &notfound && (c = ast_channel_walk_locked(c))) {
+		if (!strncasecmp(word, c->name, wordlen) && ++which > state)
+			ret = ast_strdup(c->name);
 		ast_mutex_unlock(&c->lock);
 	}
-
-	return ret;
+	return ret == &notfound ? NULL : ret;
 }
 
 static char *complete_ch_3(const char *line, const char *word, int pos, int state)
@@ -941,11 +918,23 @@
 	{ { NULL }, NULL, NULL, NULL }
 };
 
+/*! \brief initialize the _full_cmd string in * each of the builtins. */
+void ast_builtins_init(void)
+{
+	struct ast_cli_entry *e;
+
+	for (e = builtins; e->cmda[0] != NULL; e++) {
+		char buf[80];
+		ast_join(buf, sizeof(buf), e->cmda);
+		e->_full_cmd = strdup(buf);
+		if (!e->_full_cmd)
+			ast_log(LOG_WARNING, "-- cannot allocate <%s>\n", buf);
+	}
+}
+
 static struct ast_cli_entry *find_cli(char *const cmds[], int exact)
 {
-	int x;
-	int y;
-	int match;
+	int x, y, match;
 	struct ast_cli_entry *e=NULL;
 
 	for (e=helpers;e;e=e->next) {
@@ -987,32 +976,11 @@
 	return NULL;
 }
 
-static void join(char *dest, size_t destsize, char *const w[], int tws)
-{
-	ast_join(dest, destsize, w);	
-
-	if (tws && !ast_strlen_zero(dest))
-		strncat(dest, " ", destsize - strlen(dest) - 1);
-}
-
-static void join2(char *dest, size_t destsize, char *const w[])
-{
-	int x;
-	/* Join words into a string */
-	if (!dest || destsize < 1) {
-		return;
-	}
-	dest[0] = '\0';
-	for (x=0;w[x];x++) {
-		strncat(dest, w[x], destsize - strlen(dest) - 1);
-	}
-}
-
 static char *find_best(char *argv[])
 {
 	static char cmdline[80];
 	int x;
-	/* See how close we get, then print the  */
+	/* See how close we get, then print the candidate */
 	char *myargv[AST_MAX_CMD_LEN];
 	for (x=0;x<AST_MAX_CMD_LEN;x++)
 		myargv[x]=NULL;
@@ -1021,7 +989,7 @@
 		if (!find_cli(myargv, -1))
 			break;
 	}
-	join(cmdline, sizeof(cmdline), myargv, 0);
+	ast_join(cmdline, sizeof(cmdline), myargv);
 	return cmdline;
 }
 
@@ -1029,23 +997,20 @@
 {
 	struct ast_cli_entry *cur, *l=NULL;
 	ast_mutex_lock(&clilock);
-	cur = helpers;
-	while(cur) {
+	for (cur = helpers; cur; l = cur, cur = cur->next) {
 		if (e == cur) {
 			if (e->inuse) {
 				ast_log(LOG_WARNING, "Can't remove command that is in use\n");
-			} else {
-				/* Rewrite */
+			} else {	/* unlink and free dynamic storage */
 				if (l)
 					l->next = e->next;
 				else
 					helpers = e->next;
 				e->next = NULL;
-				break;
+				free(e->_full_cmd);
 			}
+			break;
 		}
-		l = cur;
-		cur = cur->next;
 	}
 	ast_mutex_unlock(&clilock);
 	return 0;
@@ -1054,47 +1019,38 @@
 int ast_cli_register(struct ast_cli_entry *e)
 {
 	struct ast_cli_entry *cur, *l=NULL;
-	char fulle[80] ="", fulltst[80] ="";
-	static int len;
+	char fulle[80] ="";
+	int lf, ret = -1;
 	
-	ast_mutex_lock(&clilock);
-	join2(fulle, sizeof(fulle), e->cmda);
+	ast_join(fulle, sizeof(fulle), e->cmda);
 	
+	ast_mutex_lock(&clilock);
 	if (find_cli(e->cmda, -1)) {
-		ast_mutex_unlock(&clilock);
 		ast_log(LOG_WARNING, "Command '%s' already registered (or something close enough)\n", fulle);
-		return -1;
+		goto done;
 	}
-	
-	for (cur = helpers; cur; cur = cur->next) {
-		join2(fulltst, sizeof(fulltst), cur->cmda);
-		len = strlen(fulltst);
-		if (strlen(fulle) < len)
-			len = strlen(fulle);
-		if (strncasecmp(fulle, fulltst, len) < 0) {
-			if (l) {
-				e->next = l->next;
-				l->next = e;
-			} else {
-				e->next = helpers;
-				helpers = e;
-			}
+	e->_full_cmd = ast_strdup(fulle);
+	if (!e->_full_cmd)
+		goto done;
+	lf = strlen(fulle);
+	for (cur = helpers; cur; l = cur, cur = cur->next) {
+		int len = strlen(cur->_full_cmd);
+		if (lf < len)
+			len = lf;
+		if (strncasecmp(fulle, cur->_full_cmd, len) < 0)
 			break;
-		}
-		l = cur;
 	}
-
-	if (!cur) {
-		if (l)
-			l->next = e;
-		else
-			helpers = e;
-		e->next = NULL;
-	}
-
+	/* goes between l and cur */
+	e->next = cur; /* could be NULL */
+	if (l)
+		l->next = e;
+	else
+		helpers = e;
+	ret = 0;	/* success */
+done:
 	ast_mutex_unlock(&clilock);
 	
-	return 0;
+	return ret;
 }
 
 /*
@@ -1116,51 +1072,72 @@
 		ast_cli_unregister(e + i);
 }
 
+/*
+ * We have two sets of commands: builtins are stored in a
+ * NULL-terminated array of ast_cli_entry, whereas external
+ * commands are in a list.
+ * When navigating, we need to keep two pointers and get
+ * the next one in lexicographic order. For the purpose,
+ * we use a structure.
+ */
+
+struct cli_iterator {
+	struct ast_cli_entry *builtins;
+	struct ast_cli_entry *helpers;
+};
+
+static struct ast_cli_entry *cli_next(struct cli_iterator *i)
+{
+	struct ast_cli_entry *e;
+
+	if (i->builtins == NULL && i->helpers == NULL) {
+		/* initialize */
+		i->builtins = builtins;
+		i->helpers = helpers;
+	}
+	e = i->builtins; /* temporary */
+	if (!e->cmda[0] || (i->helpers &&
+		    strcmp(i->helpers->_full_cmd, e->_full_cmd) < 0)) {
+		/* Use helpers */
+		e = i->helpers;
+		if (e)
+			i->helpers = e->next;
+	} else { /* use builtin. e is already set  */
+		(i->builtins)++;	/* move to next */
+	}
+	return e;
+}
+
+
 static int help_workhorse(int fd, char *match[])
 {
-	char fullcmd1[80] = "";
-	char fullcmd2[80] = "";
 	char matchstr[80];
-	char *fullcmd = NULL;
-	struct ast_cli_entry *e, *e1, *e2;
-	e1 = builtins;
-	e2 = helpers;
-	if (match)
-		join(matchstr, sizeof(matchstr), match, 0);
-	while(e1->cmda[0] || e2) {
-		if (e2)
-			join(fullcmd2, sizeof(fullcmd2), e2->cmda, 0);
-		if (e1->cmda[0])
-			join(fullcmd1, sizeof(fullcmd1), e1->cmda, 0);
-		if (!e1->cmda[0] || 
-				(e2 && (strcmp(fullcmd2, fullcmd1) < 0))) {
-			/* Use e2 */
-			e = e2;
-			fullcmd = fullcmd2;
-			/* Increment by going to next */
-			e2 = e2->next;
-		} else {
-			/* Use e1 */
-			e = e1;
-			fullcmd = fullcmd1;
-			e1++;
-		}
+	struct ast_cli_entry *e;
+	int len = 0;
+	struct cli_iterator i = { NULL, NULL};
+
+	if (match) {
+		ast_join(matchstr, sizeof(matchstr), match);
+		len = strlen(matchstr);
+	}
+	ast_mutex_lock(&clilock);
+	while( (e = cli_next(&i)) ) {
 		/* Hide commands that start with '_' */
-		if (fullcmd[0] == '_')
+		if (e->_full_cmd[0] == '_')
 			continue;
-		if (match) {
-			if (strncasecmp(matchstr, fullcmd, strlen(matchstr))) {
-				continue;
-			}
-		}
-		ast_cli(fd, "%25.25s  %s\n", fullcmd, e->summary);
+		if (match && strncasecmp(matchstr, e->_full_cmd, len))
+			continue;
+		ast_cli(fd, "%25.25s  %s\n", e->_full_cmd, e->summary);
 	}
+	ast_mutex_unlock(&clilock);
 	return 0;
 }
 
-static int handle_help(int fd, int argc, char *argv[]) {
+static int handle_help(int fd, int argc, char *argv[])
+{
 	struct ast_cli_entry *e;
 	char fullcmd[80];
+
 	if ((argc < 1))
 		return RESULT_SHOWUSAGE;
 	if (argc > 1) {
@@ -1169,14 +1146,14 @@
 			if (e->usage)
 				ast_cli(fd, "%s", e->usage);
 			else {
-				join(fullcmd, sizeof(fullcmd), argv+1, 0);
+				ast_join(fullcmd, sizeof(fullcmd), argv+1);
 				ast_cli(fd, "No help text available for '%s'.\n", fullcmd);
 			}
 		} else {
 			if (find_cli(argv + 1, -1)) {
 				return help_workhorse(fd, argv + 1);
 			} else {
-				join(fullcmd, sizeof(fullcmd), argv+1, 0);
+				ast_join(fullcmd, sizeof(fullcmd), argv+1);
 				ast_cli(fd, "No such command '%s'.\n", fullcmd);
 			}
 		}
@@ -1195,58 +1172,60 @@
 	int whitespace = 1;
 
 	*trailingwhitespace = 0;
+	if (s == NULL)	/* invalid, though! */
+		return NULL;
+	/* make a copy to store the parsed string */
 	if (!(dup = strdup(s)))
 		return NULL;
 
 	cur = dup;
-	while (!ast_strlen_zero(s)) {
-		if ((*s == '"') && !escaped) {
+	/* scan the original string copying into cur when needed */
+	for (; *s ; s++) {
+		if (x >= max - 1) {
+			ast_log(LOG_WARNING, "Too many arguments, truncating at %s\n", s);
+			break;
+		}
+		if (*s == '"' && !escaped) {
 			quoted = !quoted;
-			if (quoted & whitespace) {
-				/* If we're starting a quoted string, coming off white space, start a new argument */
-				if (x >= (max - 1)) {
-					ast_log(LOG_WARNING, "Too many arguments, truncating\n");
-					break;
-				}
+			if (quoted && whitespace) {
+				/* start a quoted string from previous whitespace: new argument */
 				argv[x++] = cur;
 				whitespace = 0;
 			}
-			escaped = 0;
-		} else if (((*s == ' ') || (*s == '\t')) && !(quoted || escaped)) {
+		} else if ((*s == ' ' || *s == '\t') && !(quoted || escaped)) {
 			/* If we are not already in whitespace, and not in a quoted string or
 			   processing an escape sequence, and just entered whitespace, then
 			   finalize the previous argument and remember that we are in whitespace
 			*/
 			if (!whitespace) {
-				*(cur++) = '\0';
+				*cur++ = '\0';
 				whitespace = 1;
 			}
-		} else if ((*s == '\\') && !escaped) {
+		} else if (*s == '\\' && !escaped) {
 			escaped = 1;
 		} else {
 			if (whitespace) {
-				/* If we are coming out of whitespace, start a new argument */
-				if (x >= (max - 1)) {
-					ast_log(LOG_WARNING, "Too many arguments, truncating\n");
-					break;
-				}
+				/* we leave whitespace, and are not quoted. So it's a new argument */
 				argv[x++] = cur;
 				whitespace = 0;
 			}
-			*(cur++) = *s;
+			*cur++ = *s;
 			escaped = 0;
 		}
-		s++;
 	}
 	/* Null terminate */
-	*(cur++) = '\0';
+	*cur++ = '\0';
+	/* XXX put a NULL in the last argument, because some functions that take
+	 * the array may want a null-terminated array.
+	 * argc still reflects the number of non-NULL entries.
+	 */
 	argv[x] = NULL;
 	*argc = x;
 	*trailingwhitespace = whitespace;
 	return dup;
 }
 
-/* This returns the number of unique matches for the generator */
+/*! \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;
@@ -1270,6 +1249,7 @@
 	size_t match_list_len, max_equal, which, i;
 	int matches = 0;
 
+	/* 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) {
@@ -1280,12 +1260,14 @@
 	}
 
 	if (!match_list)
-		return (char **) NULL;
+		return match_list; /* NULL */
 
-	which = 2;
+	/* 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 <= matches; which++) {
+	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;
@@ -1296,9 +1278,10 @@
 	retstr[max_equal] = '\0';
 	match_list[0] = retstr;
 
+	/* ensure that the array is NULL terminated */
 	if (matches + 1 >= match_list_len)
 		match_list = realloc(match_list, (match_list_len + 1) * sizeof(char *));
-	match_list[matches + 1] = (char *) NULL;
+	match_list[matches + 1] = NULL;
 
 	return match_list;
 }
@@ -1306,77 +1289,40 @@
 static char *__ast_cli_generator(const char *text, const char *word, int state, int lock)
 {
 	char *argv[AST_MAX_ARGS];
-	struct ast_cli_entry *e, *e1, *e2;
-	int x;
+	struct ast_cli_entry *e;
+	struct cli_iterator i = { NULL, NULL };
+	int x = 0, argindex, matchlen;
 	int matchnum=0;
-	char *dup, *res;
-	char fullcmd1[80] = "";
-	char fullcmd2[80] = "";
+	char *ret = NULL;
 	char matchstr[80] = "";
-	char *fullcmd = NULL;
 	int tws;
+	char *dup = parse_args(text, &x, argv, sizeof(argv) / sizeof(argv[0]), &tws);
 
-	if ((dup = parse_args(text, &x, argv, sizeof(argv) / sizeof(argv[0]), &tws))) {
-		join(matchstr, sizeof(matchstr), argv, tws);
-		if (lock)
-			ast_mutex_lock(&clilock);
-		e1 = builtins;
-		e2 = helpers;
-		while(e1->cmda[0] || e2) {
-			if (e2)
-				join(fullcmd2, sizeof(fullcmd2), e2->cmda, tws);
-			if (e1->cmda[0])
-				join(fullcmd1, sizeof(fullcmd1), e1->cmda, tws);
-			if (!e1->cmda[0] || 
-					(e2 && (strcmp(fullcmd2, fullcmd1) < 0))) {
-				/* Use e2 */
-				e = e2;
-				fullcmd = fullcmd2;
-				/* Increment by going to next */
-				e2 = e2->next;
-			} else {
-				/* Use e1 */
-				e = e1;
-				fullcmd = fullcmd1;
-				e1++;
-			}
-			if ((fullcmd[0] != '_') && !strncasecmp(matchstr, fullcmd, strlen(matchstr))) {
-				/* We contain the first part of one or more commands */
-				/* Now, what we're supposed to return is the next word... */
-				if (!ast_strlen_zero(word) && x>0) {
-					res = e->cmda[x-1];
-				} else {
-					res = e->cmda[x];
-				}
-				if (res) {
-					matchnum++;
-					if (matchnum > state) {
-						if (lock)
-							ast_mutex_unlock(&clilock);
-						free(dup);
-						return strdup(res);
-					}
-				}
-			}
-			if (e->generator && !strncasecmp(matchstr, fullcmd, strlen(fullcmd)) &&
-				(matchstr[strlen(fullcmd)] < 33)) {
-				/* We have a command in its entirity within us -- theoretically only one
-				   command can have this occur */
-				fullcmd = e->generator(matchstr, word, (!ast_strlen_zero(word) ? (x - 1) : (x)), state);
-				if (fullcmd) {
-					if (lock)
-						ast_mutex_unlock(&clilock);
-					free(dup);
-					return fullcmd;
-				}
-			}
-			
+	if (!dup)	/* error */
+		return NULL;
+	argindex = (!ast_strlen_zero(word) && x>0) ? x-1 : x;
+	/* rebuild the command, ignore tws */
+	ast_join(matchstr, sizeof(matchstr), argv);
+	matchlen = strlen(matchstr);
+	if (lock)
+		ast_mutex_lock(&clilock);
+	while( !ret && (e = cli_next(&i)) ) {
+		int lc = strlen(e->_full_cmd);
+		if (e->_full_cmd[0] != '_' && matchlen < lc &&
+				!strncasecmp(matchstr, e->_full_cmd, matchlen)) {
+			/* Found initial part, return a copy of the next word... */
+			if (e->cmda[argindex] && ++matchnum > state)
+				ret = strdup(e->cmda[argindex]); /* we need a malloced string */
+		} else if (e->generator && !strncasecmp(matchstr, e->_full_cmd, lc) && matchstr[lc] < 33) {
+			/* We have a command in its entirity within us -- theoretically only one
+			   command can have this occur */
+			ret = e->generator(matchstr, word, argindex, state);
 		}
-		if (lock)
-			ast_mutex_unlock(&clilock);
-		free(dup);
 	}
-	return NULL;
+	if (lock)
+		ast_mutex_unlock(&clilock);
+	free(dup);
+	return ret;
 }
 
 char *ast_cli_generator(const char *text, const char *word, int state)
Index: include/asterisk/cli.h
===================================================================
--- include/asterisk/cli.h	(revision 8259)
+++ include/asterisk/cli.h	(working copy)
@@ -71,8 +71,26 @@
 	struct ast_cli_entry *next;
 	/*! For keeping track of usage */
 	int inuse;
+	char *_full_cmd;	/* built at load time from cmda[] */
 };
 
+/*!
+ * \brief Helper function to generate cli entries from a NULL-terminated array.
+ * Returns the n-th matching entry from the array, or NULL if not found.
+ * Can be used to implement generate() for static entries as below
+ * (in this example we complete the word in position 2):
+  \code
+    char *my_generate(const char *line, const char *word, int pos, int n)
+    {
+        static char *choices = { "one", "two", "three", NULL };
+	if (pos == 2)
+        	return ast_cli_complete(word, choices, n);
+	else
+		return NULL;
+    }
+  \endcode
+ */
+char *ast_cli_complete(const char *word, char *const choices[], int pos);
 
 /*! \brief Interprets a command 
  * Interpret a command s, sending output to fd
Index: asterisk.c
===================================================================
--- asterisk.c	(revision 8259)
+++ asterisk.c	(working copy)
@@ -2026,6 +2086,7 @@
 	ast_ulaw_init();
 	ast_alaw_init();
 	callerid_init();
+	ast_builtins_init();
 	ast_utils_init();
 	tdd_init();
 	/* When Asterisk restarts after it has dropped the root privileges,
Index: include/asterisk.h
===================================================================
--- include/asterisk.h	(revision 8259)
+++ include/asterisk.h	(working copy)
@@ -60,10 +60,12 @@
 int astdb_init(void);
 /* Provided by channel.c */
 void ast_channels_init(void);
+/* Provided by cli.c */
+void ast_builtins_init(void);
 /* Provided by dnsmgr.c */
 int dnsmgr_init(void);
 void dnsmgr_start_refresh(void);
 void dnsmgr_reload(void);
 
 /*!
  * \brief Register the version of a source code file with the core.
