cli: Move readline character-processing to a state machine

The current cread_line() function is very long. It handles the escape
processing inline. The menu command does similar processing but at the
character level, so there is some duplication.

Split the character processing into a new function cli_ch_process() which
processes individual characters and returns the resulting input character,
taking account of escape sequences. It requires the caller to set up and
maintain its state.

Update cread_line() to use this new function.

The only intended functional change is that an invalid escape sequence
does not add invalid/control characters into the input buffer, but instead
discards these.

Signed-off-by: Simon Glass <sjg@chromium.org>

3 files changed, 229 insertions, 131 deletions

diff --git a/common/Makefile b/common/Makefile
index 7789aab484..252e9656df 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -39,7 +39,7 @@ obj-$(CONFIG_SPLASH_SOURCE) += splash_source.o
 obj-$(CONFIG_MENU) += menu.o
 obj-$(CONFIG_UPDATE_COMMON) += update.o
 obj-$(CONFIG_USB_KEYBOARD) += usb_kbd.o
-obj-$(CONFIG_CMDLINE) += cli_readline.o cli_simple.o
+obj-$(CONFIG_CMDLINE) += cli_getch.o cli_readline.o cli_simple.o
 
 endif # !CONFIG_SPL_BUILD
 
@@ -94,8 +94,8 @@ obj-y += eeprom/eeprom_field.o eeprom/eeprom_layout.o
 endif
 
 obj-y += cli.o
-obj-$(CONFIG_FSL_DDR_INTERACTIVE) += cli_simple.o cli_readline.o
-obj-$(CONFIG_STM32MP1_DDR_INTERACTIVE) += cli_simple.o cli_readline.o
+obj-$(CONFIG_FSL_DDR_INTERACTIVE) += cli_getch.o cli_simple.o cli_readline.o
+obj-$(CONFIG_STM32MP1_DDR_INTERACTIVE) += cli_getch.o cli_simple.o cli_readline.o
 obj-$(CONFIG_DFU_OVER_USB) += dfu.o
 obj-y += command.o
 obj-$(CONFIG_$(SPL_TPL_)LOG) += log.o
diff --git a/common/cli_getch.c b/common/cli_getch.c
new file mode 100644
index 0000000000..9eeea7fef2
--- /dev/null
+++ b/common/cli_getch.c
@@ -0,0 +1,204 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * (C) Copyright 2000
+ * Wolfgang Denk, DENX Software Engineering, wd@denx.de.
+ *
+ * Copyright 2022 Google LLC
+ */
+
+#include <common.h>
+#include <cli.h>
+
+/**
+ * enum cli_esc_state_t - indicates what to do with an escape character
+ *
+ * @ESC_REJECT: Invalid escape sequence, so the esc_save[] characters are
+ *	returned from each subsequent call to cli_ch_esc()
+ * @ESC_SAVE: Character should be saved in esc_save until we have another one
+ * @ESC_CONVERTED: Escape sequence has been completed and the resulting
+ *	character is available
+ */
+enum cli_esc_state_t {
+	ESC_REJECT,
+	ESC_SAVE,
+	ESC_CONVERTED
+};
+
+void cli_ch_init(struct cli_ch_state *cch)
+{
+	memset(cch, '\0', sizeof(*cch));
+}
+
+/**
+ * cli_ch_esc() - Process a character in an ongoing escape sequence
+ *
+ * @cch: State information
+ * @ichar: Character to process
+ * @actp: Returns the action to take
+ * Returns: Output character if *actp is ESC_CONVERTED, else 0
+ */
+static int cli_ch_esc(struct cli_ch_state *cch, int ichar,
+		      enum cli_esc_state_t *actp)
+{
+	enum cli_esc_state_t act = ESC_REJECT;
+
+	switch (cch->esc_len) {
+	case 1:
+		if (ichar == '[' || ichar == 'O')
+			act = ESC_SAVE;
+		break;
+	case 2:
+		switch (ichar) {
+		case 'D':	/* <- key */
+			ichar = CTL_CH('b');
+			act = ESC_CONVERTED;
+			break;	/* pass off to ^B handler */
+		case 'C':	/* -> key */
+			ichar = CTL_CH('f');
+			act = ESC_CONVERTED;
+			break;	/* pass off to ^F handler */
+		case 'H':	/* Home key */
+			ichar = CTL_CH('a');
+			act = ESC_CONVERTED;
+			break;	/* pass off to ^A handler */
+		case 'F':	/* End key */
+			ichar = CTL_CH('e');
+			act = ESC_CONVERTED;
+			break;	/* pass off to ^E handler */
+		case 'A':	/* up arrow */
+			ichar = CTL_CH('p');
+			act = ESC_CONVERTED;
+			break;	/* pass off to ^P handler */
+		case 'B':	/* down arrow */
+			ichar = CTL_CH('n');
+			act = ESC_CONVERTED;
+			break;	/* pass off to ^N handler */
+		case '1':
+		case '2':
+		case '3':
+		case '4':
+		case '7':
+		case '8':
+			if (cch->esc_save[1] == '[') {
+				/* see if next character is ~ */
+				act = ESC_SAVE;
+			}
+			break;
+		}
+		break;
+	case 3:
+		switch (ichar) {
+		case '~':
+			switch (cch->esc_save[2]) {
+			case '3':	/* Delete key */
+				ichar = CTL_CH('d');
+				act = ESC_CONVERTED;
+				break;	/* pass to ^D handler */
+			case '1':	/* Home key */
+			case '7':
+				ichar = CTL_CH('a');
+				act = ESC_CONVERTED;
+				break;	/* pass to ^A handler */
+			case '4':	/* End key */
+			case '8':
+				ichar = CTL_CH('e');
+				act = ESC_CONVERTED;
+				break;	/* pass to ^E handler */
+			}
+			break;
+		case '0':
+			if (cch->esc_save[2] == '2')
+				act = ESC_SAVE;
+			break;
+		}
+		break;
+	case 4:
+		switch (ichar) {
+		case '0':
+		case '1':
+			act = ESC_SAVE;
+			break;		/* bracketed paste */
+		}
+		break;
+	case 5:
+		if (ichar == '~') {	/* bracketed paste */
+			ichar = 0;
+			act = ESC_CONVERTED;
+		}
+	}
+
+	*actp = act;
+
+	return act == ESC_CONVERTED ? ichar : 0;
+}
+
+int cli_ch_process(struct cli_ch_state *cch, int ichar)
+{
+	/*
+	 * ichar=0x0 when error occurs in U-Boot getchar() or when the caller
+	 * wants to check if there are more characters saved in the escape
+	 * sequence
+	 */
+	if (!ichar) {
+		if (cch->emit_upto) {
+			if (cch->emit_upto < cch->esc_len)
+				return cch->esc_save[cch->emit_upto++];
+			cch->emit_upto = 0;
+		}
+		return 0;
+	} else if (ichar == -ETIMEDOUT) {
+		/*
+		 * If we are in an escape sequence but nothing has followed the
+		 * Escape character, then the user probably just pressed the
+		 * Escape key. Return it and clear the sequence.
+		 */
+		if (cch->esc_len) {
+			cch->esc_len = 0;
+			return '\e';
+		}
+
+		/* Otherwise there is nothing to return */
+		return 0;
+	}
+
+	if (ichar == '\n' || ichar == '\r')
+		return '\n';
+
+	/* handle standard linux xterm esc sequences for arrow key, etc. */
+	if (cch->esc_len != 0) {
+		enum cli_esc_state_t act;
+
+		ichar = cli_ch_esc(cch, ichar, &act);
+
+		switch (act) {
+		case ESC_SAVE:
+			/* save this character and return nothing */
+			cch->esc_save[cch->esc_len++] = ichar;
+			return 0;
+		case ESC_REJECT:
+			/*
+			 * invalid escape sequence, start returning the
+			 * characters in it
+			 */
+			cch->esc_save[cch->esc_len++] = ichar;
+			return cch->esc_save[cch->emit_upto++];
+		case ESC_CONVERTED:
+			/* valid escape sequence, return the resulting char */
+			cch->esc_len = 0;
+			return ichar;
+		}
+	}
+
+	if (ichar == '\e') {
+		if (!cch->esc_len) {
+			cch->esc_save[cch->esc_len] = ichar;
+			cch->esc_len = 1;
+		} else {
+			puts("impossible condition #876\n");
+			cch->esc_len = 0;
+		}
+		return 0;
+	}
+
+	return ichar;
+}
diff --git a/common/cli_readline.c b/common/cli_readline.c
index d6444f5fc1..709e9c3d38 100644
--- a/common/cli_readline.c
+++ b/common/cli_readline.c
@@ -62,7 +62,6 @@ static char *delete_char (char *buffer, char *p, int *colp, int *np, int plen)
 
 #define putnstr(str, n)	printf("%.*s", (int)n, str)
 
-#define CTL_CH(c)		((c) - 'a' + 1)
 #define CTL_BACKSPACE		('\b')
 #define DEL			((char)255)
 #define DEL7			((char)127)
@@ -252,156 +251,53 @@ static void cread_add_str(char *str, int strsize, int insert,
 static int cread_line(const char *const prompt, char *buf, unsigned int *len,
 		int timeout)
 {
+	struct cli_ch_state s_cch, *cch = &s_cch;
 	unsigned long num = 0;
 	unsigned long eol_num = 0;
 	unsigned long wlen;
 	char ichar;
 	int insert = 1;
-	int esc_len = 0;
-	char esc_save[8];
 	int init_len = strlen(buf);
 	int first = 1;
 
+	cli_ch_init(cch);
+
 	if (init_len)
 		cread_add_str(buf, init_len, 1, &num, &eol_num, buf, *len);
 
 	while (1) {
-		if (bootretry_tstc_timeout())
-			return -2;	/* timed out */
-		if (first && timeout) {
-			uint64_t etime = endtick(timeout);
-
-			while (!tstc()) {	/* while no incoming data */
-				if (get_ticks() >= etime)
-					return -2;	/* timed out */
-				schedule();
+		/* Check for saved characters */
+		ichar = cli_ch_process(cch, 0);
+
+		if (!ichar) {
+			if (bootretry_tstc_timeout())
+				return -2;	/* timed out */
+			if (first && timeout) {
+				u64 etime = endtick(timeout);
+
+				while (!tstc()) {	/* while no incoming data */
+					if (get_ticks() >= etime)
+						return -2;	/* timed out */
+					schedule();
+				}
+				first = 0;
 			}
-			first = 0;
+
+			ichar = getcmd_getch();
 		}
 
-		ichar = getcmd_getch();
+		ichar = cli_ch_process(cch, ichar);
 
 		/* ichar=0x0 when error occurs in U-Boot getc */
 		if (!ichar)
 			continue;
 
-		if ((ichar == '\n') || (ichar == '\r')) {
+		if (ichar == '\n') {
 			putc('\n');
 			break;
 		}
 
-		/*
-		 * handle standard linux xterm esc sequences for arrow key, etc.
-		 */
-		if (esc_len != 0) {
-			enum { ESC_REJECT, ESC_SAVE, ESC_CONVERTED } act = ESC_REJECT;
-
-			if (esc_len == 1) {
-				if (ichar == '[' || ichar == 'O')
-					act = ESC_SAVE;
-			} else if (esc_len == 2) {
-				switch (ichar) {
-				case 'D':	/* <- key */
-					ichar = CTL_CH('b');
-					act = ESC_CONVERTED;
-					break;	/* pass off to ^B handler */
-				case 'C':	/* -> key */
-					ichar = CTL_CH('f');
-					act = ESC_CONVERTED;
-					break;	/* pass off to ^F handler */
-				case 'H':	/* Home key */
-					ichar = CTL_CH('a');
-					act = ESC_CONVERTED;
-					break;	/* pass off to ^A handler */
-				case 'F':	/* End key */
-					ichar = CTL_CH('e');
-					act = ESC_CONVERTED;
-					break;	/* pass off to ^E handler */
-				case 'A':	/* up arrow */
-					ichar = CTL_CH('p');
-					act = ESC_CONVERTED;
-					break;	/* pass off to ^P handler */
-				case 'B':	/* down arrow */
-					ichar = CTL_CH('n');
-					act = ESC_CONVERTED;
-					break;	/* pass off to ^N handler */
-				case '1':
-				case '2':
-				case '3':
-				case '4':
-				case '7':
-				case '8':
-					if (esc_save[1] == '[') {
-						/* see if next character is ~ */
-						act = ESC_SAVE;
-					}
-					break;
-				}
-			} else if (esc_len == 3) {
-				switch (ichar) {
-				case '~':
-					switch (esc_save[2]) {
-					case '3':	/* Delete key */
-						ichar = CTL_CH('d');
-						act = ESC_CONVERTED;
-						break;	/* pass to ^D handler */
-					case '1':	/* Home key */
-					case '7':
-						ichar = CTL_CH('a');
-						act = ESC_CONVERTED;
-						break;	/* pass to ^A handler */
-					case '4':	/* End key */
-					case '8':
-						ichar = CTL_CH('e');
-						act = ESC_CONVERTED;
-						break;	/* pass to ^E handler */
-					}
-					break;
-				case '0':
-					if (esc_save[2] == '2')
-						act = ESC_SAVE;
-					break;
-				}
-			} else if (esc_len == 4) {
-				switch (ichar) {
-				case '0':
-				case '1':
-					act = ESC_SAVE;
-					break;		/* bracketed paste */
-				}
-			} else if (esc_len == 5) {
-				if (ichar == '~') {	/* bracketed paste */
-					ichar = 0;
-					act = ESC_CONVERTED;
-				}
-			}
-			switch (act) {
-			case ESC_SAVE:
-				esc_save[esc_len++] = ichar;
-				continue;
-			case ESC_REJECT:
-				esc_save[esc_len++] = ichar;
-				cread_add_str(esc_save, esc_len, insert,
-					      &num, &eol_num, buf, *len);
-				esc_len = 0;
-				continue;
-			case ESC_CONVERTED:
-				esc_len = 0;
-				break;
-			}
-		}
-
 		switch (ichar) {
-		case 0x1b:
-			if (esc_len == 0) {
-				esc_save[esc_len] = ichar;
-				esc_len = 1;
-			} else {
-				puts("impossible condition #876\n");
-				esc_len = 0;
-			}
-			break;
-
 		case CTL_CH('a'):
 			BEGINNING_OF_LINE();
 			break;
@@ -470,8 +366,6 @@ static int cread_line(const char *const prompt, char *buf, unsigned int *len,
 		{
 			char *hline;
 
-			esc_len = 0;
-
 			if (ichar == CTL_CH('p'))
 				hline = hist_prev();
 			else