All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] cli: allow verbatim character entry with CTRL-v
@ 2020-01-12 16:33 Heinrich Schuchardt
  2020-01-12 16:33 ` [PATCH 1/2] " Heinrich Schuchardt
  2020-01-12 16:33 ` [PATCH 2/2] test: verbatim character entry with CTRL-V Heinrich Schuchardt
  0 siblings, 2 replies; 7+ messages in thread
From: Heinrich Schuchardt @ 2020-01-12 16:33 UTC (permalink / raw)
  To: u-boot

Up to now the U-Boot console does not allow to add a control character to
the line buffer. In Bash and Vim we can use CTRL-v for this purpose.

Add support for CTRL-v for verbatim entry of characters.

Heinrich Schuchardt (2):
  cli: allow verbatim character entry with CTRL-v
  test: verbatim character entry with CTRL-V

 common/cli_readline.c |  38 ++++++++++-
 test/dm/Makefile      |   2 +-
 test/dm/usb_console.c | 151 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 187 insertions(+), 4 deletions(-)
 create mode 100644 test/dm/usb_console.c

--
2.24.1

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/2] cli: allow verbatim character entry with CTRL-v
  2020-01-12 16:33 [PATCH 0/2] cli: allow verbatim character entry with CTRL-v Heinrich Schuchardt
@ 2020-01-12 16:33 ` Heinrich Schuchardt
  2020-01-30  2:17   ` Simon Glass
  2020-01-12 16:33 ` [PATCH 2/2] test: verbatim character entry with CTRL-V Heinrich Schuchardt
  1 sibling, 1 reply; 7+ messages in thread
From: Heinrich Schuchardt @ 2020-01-12 16:33 UTC (permalink / raw)
  To: u-boot

Up to now the U-Boot console does not allow to add a control character to
the line buffer. In Bash and Vim we can use CTRL-v for this purpose.

Add support for CTRL-v for verbatim entry of characters.

To keep things simple, while editing control characters are displayed as
simple characters, e.g. ESC (0x1b) is displayed as [ and not as ^[.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 common/cli_readline.c | 38 +++++++++++++++++++++++++++++++++++---
 1 file changed, 35 insertions(+), 3 deletions(-)

diff --git a/common/cli_readline.c b/common/cli_readline.c
index 6ef7a3e564..402f15a361 100644
--- a/common/cli_readline.c
+++ b/common/cli_readline.c
@@ -58,8 +58,6 @@ static char *delete_char (char *buffer, char *p, int *colp, int *np, int plen)
  * Author: Janghoon Lyu <nandy@mizi.com>
  */

-#define putnstr(str, n)	printf("%.*s", (int)n, str)
-
 #define CTL_CH(c)		((c) - 'a' + 1)
 #define CTL_BACKSPACE		('\b')
 #define DEL			((char)255)
@@ -83,6 +81,34 @@ static char hist_lines[HIST_MAX][HIST_SIZE + 1];	/* Save room for NULL */

 #define add_idx_minus_one() ((hist_add_idx == 0) ? hist_max : hist_add_idx-1)

+/**
+ * putchar() - output single character
+ *
+ * Outputs single character. Control characters are converted,
+ * e.g. CTR-c is displayed as C.
+ */
+static void putchar(char c)
+{
+	if (c < ' ')
+		c += '@';
+	putc(c);
+}
+
+/**
+ * putnstr() - output string buffer
+ *
+ * Outputs n characters for buffer str. Control characters are converted, e.g.
+ * CTRL-c is displayed as C.
+ *
+ * @str:	string buffer
+ * @n:		number of characters to print
+ */
+static void putnstr(const char *str, size_t n)
+{
+	for (; n; --n)
+		putchar(*str++);
+}
+
 static void hist_init(void)
 {
 	int i;
@@ -385,7 +411,7 @@ static int cread_line(const char *const prompt, char *buf, unsigned int *len,
 			return -1;
 		case CTL_CH('f'):
 			if (num < eol_num) {
-				getcmd_putch(buf[num]);
+				putchar(buf[num]);
 				num++;
 			}
 			break;
@@ -419,6 +445,12 @@ static int cread_line(const char *const prompt, char *buf, unsigned int *len,
 		case CTL_CH('o'):
 			insert = !insert;
 			break;
+		case CTL_CH('v'):
+			/* Add next character verbatim */
+			ichar = getcmd_getch();
+			cread_add_char(ichar, insert, &num, &eol_num, buf,
+				       *len);
+			break;
 		case CTL_CH('x'):
 		case CTL_CH('u'):
 			BEGINNING_OF_LINE();
--
2.24.1

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/2] test: verbatim character entry with CTRL-V
  2020-01-12 16:33 [PATCH 0/2] cli: allow verbatim character entry with CTRL-v Heinrich Schuchardt
  2020-01-12 16:33 ` [PATCH 1/2] " Heinrich Schuchardt
@ 2020-01-12 16:33 ` Heinrich Schuchardt
  2020-01-24 23:43   ` Tom Rini
  1 sibling, 1 reply; 7+ messages in thread
From: Heinrich Schuchardt @ 2020-01-12 16:33 UTC (permalink / raw)
  To: u-boot

Provide a unit test checking that CTRL-V can be used to add control
characters to the line buffer.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 test/dm/Makefile      |   2 +-
 test/dm/usb_console.c | 150 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 151 insertions(+), 1 deletion(-)
 create mode 100644 test/dm/usb_console.c

diff --git a/test/dm/Makefile b/test/dm/Makefile
index dd1ceff86c..0261424168 100644
--- a/test/dm/Makefile
+++ b/test/dm/Makefile
@@ -51,7 +51,7 @@ obj-$(CONFIG_DM_SPI_FLASH) += sf.o
 obj-$(CONFIG_SMEM) += smem.o
 obj-$(CONFIG_DM_SPI) += spi.o
 obj-y += syscon.o
-obj-$(CONFIG_DM_USB) += usb.o
+obj-$(CONFIG_DM_USB) += usb.o usb_console.o
 obj-$(CONFIG_DM_PMIC) += pmic.o
 obj-$(CONFIG_DM_REGULATOR) += regulator.o
 obj-$(CONFIG_TIMER) += timer.o
diff --git a/test/dm/usb_console.c b/test/dm/usb_console.c
new file mode 100644
index 0000000000..c62a05291d
--- /dev/null
+++ b/test/dm/usb_console.c
@@ -0,0 +1,150 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2019 Heinrich Schuchardt <xypron.glpk@gmx.de>
+ *
+ * Tests for the command line interface using the USB keyboard as input.
+ */
+
+#include <common.h>
+#include <cli.h>
+#include <asm/state.h>
+#include <asm/test.h>
+#include <dm/test.h>
+#include <test/ut.h>
+#include <usb.h>
+
+#define SCAN_CODE_END_OF_BUFFER 0xff
+
+struct console_test_data {
+	const char modifiers;
+	const unsigned char scancode;
+};
+
+/**
+ * put_buffer() - copy key strokes to USB keyboard buffer
+ *
+ * @key_strokes:	key strokes
+ * Return:		0 = ok
+ */
+static int put_buffer(const struct console_test_data *key_strokes,
+		      struct unit_test_state *uts)
+{
+	struct udevice *dev;
+	const struct console_test_data *pos;
+
+	ut_assertok(uclass_get_device_by_name(UCLASS_USB_EMUL, "keyb at 3",
+					      &dev));
+	for (pos = key_strokes; pos->scancode != SCAN_CODE_END_OF_BUFFER;
+	     ++pos) {
+		char scancodes[USB_KBD_BOOT_REPORT_SIZE] = {0};
+
+		scancodes[0] = pos->modifiers;
+		scancodes[2] = pos->scancode;
+
+		ut_assertok(sandbox_usb_keyb_add_string(dev, scancodes));
+	}
+	ut_asserteq(1, tstc());
+
+	return 0;
+}
+
+/**
+ * usb_console_setup() - setup for usb keyboard test
+ *
+ * Return:	0 = ok
+ */
+static int usb_console_setup(struct unit_test_state *uts)
+{
+	state_set_skip_delays(true);
+	ut_assertok(usb_init());
+
+	/* Initially there should be no characters */
+	ut_asserteq(0, tstc());
+
+	return 0;
+}
+
+/**
+ * usb_console_teardown() - tear down after usb keyboard test
+ *
+ * Return:	0 = ok
+ */
+static int usb_console_teardown(struct unit_test_state *uts)
+{
+	ut_assertok(usb_stop());
+	return 0;
+}
+
+/**
+ * dm_test_usb_console_verbatim() - test verbatim entry in console
+ *
+ * This test copies the key strokes
+ *
+ *	setenv error '<CTRL-V><ESC>[33;1mError<CTRL-V><ESC>[0m'<ENTER>
+ *
+ * into the buffer of the USB keyboard emulation driver. It then checks if the
+ * string is read into a command line interface buffer as
+ *
+ *	"setenv error '\x1b[33;1mError\x1b[0m'"
+ *
+ * This confirms that <CTRL-V> can be used for verbatim input of control
+ * characters.
+ *
+ * @uts:	unit test state
+ * Return:	0 on success
+ */
+static int dm_test_usb_console_verbatim(struct unit_test_state *uts)
+{
+	char buffer[CONFIG_SYS_CBSIZE + 1]; /* console I/O buffer */
+	const struct console_test_data con_test_data[] = {
+		{0x00, 0x16}, /* s */
+		{0x00, 0x08}, /* e */
+		{0x00, 0x17}, /* t */
+		{0x00, 0x08}, /* e */
+		{0x00, 0x11}, /* n */
+		{0x00, 0x19}, /* v */
+		{0x00, 0x2c}, /*   */
+		{0x00, 0x08}, /* e */
+		{0x00, 0x15}, /* r */
+		{0x00, 0x00}, /*   */
+		{0x00, 0x15}, /* r */
+		{0x00, 0x12}, /* o */
+		{0x00, 0x15}, /* r */
+		{0x00, 0x2c}, /*   */
+		{0x00, 0x34}, /* ' */
+		{0x01, 0x19}, /* <CONTROL><V> */
+		{0x00, 0x29}, /* <ESC> */
+		{0x00, 0x2F}, /* [ */
+		{0x00, 0x20}, /* 3 */
+		{0x00, 0x00}, /*   */
+		{0x00, 0x20}, /* 3 */
+		{0x00, 0x33}, /* ; */
+		{0x00, 0x1E}, /* 1 */
+		{0x00, 0x10}, /* m */
+		{0x20, 0x08}, /* E */
+		{0x00, 0x15}, /* r */
+		{0x00, 0x00}, /*   */
+		{0x00, 0x15}, /* r */
+		{0x00, 0x12}, /* o */
+		{0x00, 0x15}, /* r */
+		{0x01, 0x19}, /* <CONTROL><V> */
+		{0x00, 0x29}, /* <ESC> */
+		{0x00, 0x2F}, /* [ */
+		{0x00, 0x27}, /* 0 */
+		{0x00, 0x10}, /* m */
+		{0x00, 0x34}, /* ' */
+		{0x00, 0x28}, /* <ENTER> */
+		/* End of list */
+		{0x00, SCAN_CODE_END_OF_BUFFER}
+	};
+
+	ut_assertok(usb_console_setup(uts));
+	ut_assertok(put_buffer(con_test_data, uts));
+	ut_asserteq(31, cli_readline_into_buffer("", buffer, 0));
+	ut_asserteq_str("setenv error '\x1b[33;1mError\x1b[0m'", buffer);
+	ut_asserteq(0, tstc());
+	ut_assertok(usb_console_teardown(uts));
+
+	return 0;
+}
+DM_TEST(dm_test_usb_console_verbatim, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
--
2.24.1

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/2] test: verbatim character entry with CTRL-V
  2020-01-12 16:33 ` [PATCH 2/2] test: verbatim character entry with CTRL-V Heinrich Schuchardt
@ 2020-01-24 23:43   ` Tom Rini
  2020-01-25 10:45     ` Heinrich Schuchardt
  0 siblings, 1 reply; 7+ messages in thread
From: Tom Rini @ 2020-01-24 23:43 UTC (permalink / raw)
  To: u-boot

On Sun, Jan 12, 2020 at 05:33:39PM +0100, Heinrich Schuchardt wrote:

> Provide a unit test checking that CTRL-V can be used to add control
> characters to the line buffer.
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

I don't know why but this test, run when built with clang fails.  A log
is here: https://gitlab.denx.de/u-boot/u-boot/-/jobs/50540 but I saw it
on Travis and Azure as well.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200124/b1dbbca3/attachment.sig>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 2/2] test: verbatim character entry with CTRL-V
  2020-01-24 23:43   ` Tom Rini
@ 2020-01-25 10:45     ` Heinrich Schuchardt
  2020-03-14 20:35       ` Simon Glass
  0 siblings, 1 reply; 7+ messages in thread
From: Heinrich Schuchardt @ 2020-01-25 10:45 UTC (permalink / raw)
  To: u-boot

On 1/25/20 12:43 AM, Tom Rini wrote:
> On Sun, Jan 12, 2020 at 05:33:39PM +0100, Heinrich Schuchardt wrote:
>
>> Provide a unit test checking that CTRL-V can be used to add control
>> characters to the line buffer.
>>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>
> I don't know why but this test, run when built with clang fails.  A log
> is here: https://gitlab.denx.de/u-boot/u-boot/-/jobs/50540 but I saw it
> on Travis and Azure as well.
>

Hello Tom, hello Simon

when I clang U-Boot branch bisect-failure with in the Gitlab Docker
image and run

./u-boot -d arch/sandbox/dts/test.dtb
ut dm usb_keyb

the test runs fine.

When I run

valgrind ./u-boot -d arch/sandbox/dts/test.dtb
usb start
ut dm usb_keyb

I see more than 100 use-after-free errors.

dm_test_destroy() calling uclass_destroy() seems to be the culprit.

@Simon:
Why would we destroy any uclass in `ut dm` that was in use before
executing the command?

@Marek
Aren't usb_kbd_remove() and usb_kbd_deregister() essentially duplicate
coding?

==149271== Invalid write of size 4
==149271==    at 0x42CC04: usb_kbd_put_queue (usb_kbd.c:0)
==149271==    by 0x42CC04: usb_kbd_translate (usb_kbd.c:255)
==149271==    by 0x42C871: usb_kbd_irq_worker (usb_kbd.c:321)
==149271==    by 0x42C7DF: usb_kbd_poll_for_event (usb_kbd.c:358)
==149271==    by 0x42C7DF: usb_kbd_testc (usb_kbd.c:410)
==149271==    by 0x42EF60: console_tstc (console.c:206)
==149271==    by 0x50F302: dm_test_usb_keyb (usb.c:0)
==149271==    by 0x4F17B4: dm_do_test (test-main.c:103)
==149271==    by 0x4F1329: dm_test_main (test-main.c:180)
==149271==    by 0x4F1329: do_ut_dm (test-main.c:210)
==149271==    by 0x437A3B: cmd_call (command.c:576)
==149271==    by 0x437A3B: cmd_process (command.c:631)
==149271==    by 0x423DF2: run_pipe_real (cli_hush.c:1678)
==149271==    by 0x423DF2: run_list_real (cli_hush.c:1876)
==149271==    by 0x422FE2: run_list (cli_hush.c:2025)
==149271==    by 0x422FE2: parse_stream_outer (cli_hush.c:3217)
==149271==    by 0x423207: parse_file_outer (cli_hush.c:3300)
==149271==    by 0x436EAA: cli_loop (cli.c:221)
==149271==  Address 0xa681ca4 is 36 bytes inside a block of size 104 free'd
==149271==    at 0x4C30D3B: free (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==149271==    by 0x42C6C9: usb_kbd_remove (usb_kbd.c:676)
==149271==    by 0x442947: device_remove (device-remove.c:184)
==149271==    by 0x442910: device_chld_remove (device-remove.c:52)
==149271==    by 0x442910: device_remove (device-remove.c:175)
==149271==    by 0x442910: device_chld_remove (device-remove.c:52)
==149271==    by 0x442910: device_remove (device-remove.c:175)
==149271==    by 0x442910: device_chld_remove (device-remove.c:52)
==149271==    by 0x442910: device_remove (device-remove.c:175)
==149271==    by 0x4415DC: uclass_destroy (uclass.c:121)
==149271==    by 0x4F17DA: dm_test_destroy (test-main.c:72)
==149271==    by 0x4F17DA: dm_do_test (test-main.c:107)
==149271==    by 0x4F12CB: dm_test_main (test-main.c:169)
==149271==    by 0x4F12CB: do_ut_dm (test-main.c:210)
==149271==    by 0x437A3B: cmd_call (command.c:576)
==149271==    by 0x437A3B: cmd_process (command.c:631)
==149271==    by 0x423DF2: run_pipe_real (cli_hush.c:1678)
==149271==    by 0x423DF2: run_list_real (cli_hush.c:1876)
==149271==    by 0x422FE2: run_list (cli_hush.c:2025)
==149271==    by 0x422FE2: parse_stream_outer (cli_hush.c:3217)


Best regards

Heinrich

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/2] cli: allow verbatim character entry with CTRL-v
  2020-01-12 16:33 ` [PATCH 1/2] " Heinrich Schuchardt
@ 2020-01-30  2:17   ` Simon Glass
  0 siblings, 0 replies; 7+ messages in thread
From: Simon Glass @ 2020-01-30  2:17 UTC (permalink / raw)
  To: u-boot

On Sun, 12 Jan 2020 at 09:33, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> Up to now the U-Boot console does not allow to add a control character to
> the line buffer. In Bash and Vim we can use CTRL-v for this purpose.
>
> Add support for CTRL-v for verbatim entry of characters.
>
> To keep things simple, while editing control characters are displayed as
> simple characters, e.g. ESC (0x1b) is displayed as [ and not as ^[.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  common/cli_readline.c | 38 +++++++++++++++++++++++++++++++++++---
>  1 file changed, 35 insertions(+), 3 deletions(-)

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 2/2] test: verbatim character entry with CTRL-V
  2020-01-25 10:45     ` Heinrich Schuchardt
@ 2020-03-14 20:35       ` Simon Glass
  0 siblings, 0 replies; 7+ messages in thread
From: Simon Glass @ 2020-03-14 20:35 UTC (permalink / raw)
  To: u-boot

Hi Heinrich,

On Sat, 25 Jan 2020 at 03:45, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 1/25/20 12:43 AM, Tom Rini wrote:
> > On Sun, Jan 12, 2020 at 05:33:39PM +0100, Heinrich Schuchardt wrote:
> >
> >> Provide a unit test checking that CTRL-V can be used to add control
> >> characters to the line buffer.
> >>
> >> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> >
> > I don't know why but this test, run when built with clang fails.  A log
> > is here: https://gitlab.denx.de/u-boot/u-boot/-/jobs/50540 but I saw it
> > on Travis and Azure as well.
> >
>
> Hello Tom, hello Simon
>
> when I clang U-Boot branch bisect-failure with in the Gitlab Docker
> image and run
>
> ./u-boot -d arch/sandbox/dts/test.dtb
> ut dm usb_keyb
>
> the test runs fine.
>
> When I run
>
> valgrind ./u-boot -d arch/sandbox/dts/test.dtb
> usb start
> ut dm usb_keyb
>
> I see more than 100 use-after-free errors.
>
> dm_test_destroy() calling uclass_destroy() seems to be the culprit.
>
> @Simon:
> Why would we destroy any uclass in `ut dm` that was in use before
> executing the command?

This is because the tests might require them to be in their initial state.

In general running a unit test along with other code is not safe.

I wonder if the situation has improved with the move to SDL2, but I'm not sure.
[..]

Regards,
Simon

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-03-14 20:35 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-12 16:33 [PATCH 0/2] cli: allow verbatim character entry with CTRL-v Heinrich Schuchardt
2020-01-12 16:33 ` [PATCH 1/2] " Heinrich Schuchardt
2020-01-30  2:17   ` Simon Glass
2020-01-12 16:33 ` [PATCH 2/2] test: verbatim character entry with CTRL-V Heinrich Schuchardt
2020-01-24 23:43   ` Tom Rini
2020-01-25 10:45     ` Heinrich Schuchardt
2020-03-14 20:35       ` Simon Glass

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.