Merge branch '2023-03-28-correct-several-cli_getch-bugs'

- Merge a regression fix (for an issue that we raised post v2023.01)
  with cli_getch(), and include a test now.

5 files changed, 100 insertions, 6 deletions

diff --git a/common/cli_getch.c b/common/cli_getch.c
index 87c23edcf4..61d4cb261b 100644
--- a/common/cli_getch.c
+++ b/common/cli_getch.c
@@ -129,7 +129,7 @@ static int cli_ch_esc(struct cli_ch_state *cch, int ichar,
 
 	*actp = act;
 
-	return act == ESC_CONVERTED ? ichar : 0;
+	return ichar;
 }
 
 int cli_ch_process(struct cli_ch_state *cch, int ichar)
@@ -145,6 +145,7 @@ int cli_ch_process(struct cli_ch_state *cch, int ichar)
 				return cch->esc_save[cch->emit_upto++];
 			cch->emit_upto = 0;
 			cch->emitting = false;
+			cch->esc_len = 0;
 		}
 		return 0;
 	} else if (ichar == -ETIMEDOUT) {
@@ -185,7 +186,7 @@ int cli_ch_process(struct cli_ch_state *cch, int ichar)
 			cch->esc_save[cch->esc_len++] = ichar;
 			ichar = cch->esc_save[cch->emit_upto++];
 			cch->emitting = true;
-			break;
+			return ichar;
 		case ESC_CONVERTED:
 			/* valid escape sequence, return the resulting char */
 			cch->esc_len = 0;
diff --git a/common/cli_readline.c b/common/cli_readline.c
index 709e9c3d38..e83743e90c 100644
--- a/common/cli_readline.c
+++ b/common/cli_readline.c
@@ -284,10 +284,9 @@ static int cread_line(const char *const prompt, char *buf, unsigned int *len,
 			}
 
 			ichar = getcmd_getch();
+			ichar = cli_ch_process(cch, ichar);
 		}
 
-		ichar = cli_ch_process(cch, ichar);
-
 		/* ichar=0x0 when error occurs in U-Boot getc */
 		if (!ichar)
 			continue;
diff --git a/include/cli.h b/include/cli.h
index c777c90313..094a6602d7 100644
--- a/include/cli.h
+++ b/include/cli.h
@@ -98,8 +98,8 @@ int cli_readline(const char *const prompt);
  *
  * @prompt:	Prompt to display
  * @buffer:	Place to put the line that is entered
- * @timeout:	Timeout in milliseconds, 0 if none
- * Return: command line length excluding terminator, or -ve on error: of the
+ * @timeout:	Timeout in seconds, 0 if none
+ * Return: command line length excluding terminator, or -ve on error: if the
  * timeout is exceeded (either CONFIG_BOOT_RETRY_TIME or the timeout
  * parameter), then -2 is returned. If a break is detected (Ctrl-C) then
  * -1 is returned.
diff --git a/test/common/Makefile b/test/common/Makefile
index cc918f64e5..a5ab10f6f6 100644
--- a/test/common/Makefile
+++ b/test/common/Makefile
@@ -3,3 +3,4 @@ obj-y += cmd_ut_common.o
 obj-$(CONFIG_AUTOBOOT) += test_autoboot.o
 obj-$(CONFIG_CYCLIC) += cyclic.o
 obj-$(CONFIG_EVENT) += event.o
+obj-y += cread.o
diff --git a/test/common/cread.c b/test/common/cread.c
new file mode 100644
index 0000000000..2fdd29a265
--- /dev/null
+++ b/test/common/cread.c
@@ -0,0 +1,93 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2023 Google LLC
+ */
+
+#include <common.h>
+#include <cli.h>
+#include <test/common.h>
+#include <test/test.h>
+#include <test/ut.h>
+
+static int cli_ch_test(struct unit_test_state *uts)
+{
+	struct cli_ch_state s_cch, *cch = &s_cch;
+
+	cli_ch_init(cch);
+
+	/* should be nothing to return at first */
+	ut_asserteq(0, cli_ch_process(cch, 0));
+
+	/* check normal entry */
+	ut_asserteq('a', cli_ch_process(cch, 'a'));
+	ut_asserteq('b', cli_ch_process(cch, 'b'));
+	ut_asserteq('c', cli_ch_process(cch, 'c'));
+	ut_asserteq(0, cli_ch_process(cch, 0));
+
+	/* send an invalid escape sequence */
+	ut_asserteq(0, cli_ch_process(cch, '\e'));
+	ut_asserteq(0, cli_ch_process(cch, '['));
+
+	/*
+	 * with the next char it sees that the sequence is invalid, so starts
+	 * emitting it
+	 */
+	ut_asserteq('\e', cli_ch_process(cch, 'X'));
+
+	/* now we set 0 bytes to empty the buffer */
+	ut_asserteq('[', cli_ch_process(cch, 0));
+	ut_asserteq('X', cli_ch_process(cch, 0));
+	ut_asserteq(0, cli_ch_process(cch, 0));
+
+	/* things are normal again */
+	ut_asserteq('a', cli_ch_process(cch, 'a'));
+	ut_asserteq(0, cli_ch_process(cch, 0));
+
+	return 0;
+}
+COMMON_TEST(cli_ch_test, 0);
+
+static int cread_test(struct unit_test_state *uts)
+{
+	int duration;
+	ulong start;
+	char buf[10];
+
+	/*
+	 * useful for debugging
+	 *
+	 * gd->flags &= ~GD_FLG_RECORD;
+	 * print_buffer(0, buf, 1, 7, 0);
+	 */
+
+	console_record_reset_enable();
+
+	/* simple input */
+	*buf = '\0';
+	ut_asserteq(4, console_in_puts("abc\n"));
+	ut_asserteq(3, cli_readline_into_buffer("-> ", buf, 1));
+	ut_asserteq_str("abc", buf);
+
+	/* try an escape sequence (cursor left after the 'c') */
+	*buf = '\0';
+	ut_asserteq(8, console_in_puts("abc\e[Dx\n"));
+	ut_asserteq(4, cli_readline_into_buffer("-> ", buf, 1));
+	ut_asserteq_str("abxc", buf);
+
+	/* invalid escape sequence */
+	*buf = '\0';
+	ut_asserteq(8, console_in_puts("abc\e[Xx\n"));
+	ut_asserteq(7, cli_readline_into_buffer("-> ", buf, 1));
+	ut_asserteq_str("abc\e[Xx", buf);
+
+	/* check timeout, should be between 1000 and 1050ms */
+	start = get_timer(0);
+	*buf = '\0';
+	ut_asserteq(-2, cli_readline_into_buffer("-> ", buf, 1));
+	duration = get_timer(start) - 1000;
+	ut_assert(duration >= 0);
+	ut_assert(duration < 50);
+
+	return 0;
+}
+COMMON_TEST(cread_test, 0);