[PATCH v3 02/25] cli: Move readline character-processing to a state machine
Heinrich Schuchardt
xypron.glpk at gmx.de
Fri Jan 6 16:50:24 CET 2023
On 1/6/23 15:52, Simon Glass wrote:
> 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 at chromium.org>
> ---
>
> (no changes since v1)
>
> common/Makefile | 6 +-
> common/cli_getch.c | 204 ++++++++++++++++++++++++++++++++++++++++++
> common/cli_readline.c | 150 +++++--------------------------
> include/cli.h | 72 +++++++++++++++
> 4 files changed, 301 insertions(+), 131 deletions(-)
> create mode 100644 common/cli_getch.c
>
> diff --git a/common/Makefile b/common/Makefile
> index 20addfb244c..67485e77a04 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -38,7 +38,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
>
> @@ -93,8 +93,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 00000000000..9eeea7fef29
> --- /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 at 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 d6444f5fc1d..709e9c3d38b 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 */
We have similar code to decode escape sequences and Unicode letters in
efi_cin_read_key_stroke_ex(). The big difference in the EFI code is that
it does not use dummy control characters but uses a structure:
struct efi_key_data {
struct efi_input_key key;
struct efi_key_state key_state;
};
with
struct efi_input_key {
u16 scan_code;
s16 unicode_char;
};
The special keys are represented as numbers in scan_code while
characters are represented in unicode_char.
Separating special keys from characters is much cleaner than creating
dummy control characters.
Anyway you would be able to first create the structure above and then in
a separate step to convert to control characters.
The code below misses to interprete some special keys like FN1 - FN12
which could be quite useful for navigating a UI.
Best regards
Heinrich
> - 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
> diff --git a/include/cli.h b/include/cli.h
> index ba5b8ebd36e..863519e4b13 100644
> --- a/include/cli.h
> +++ b/include/cli.h
> @@ -7,6 +7,21 @@
> #ifndef __CLI_H
> #define __CLI_H
>
> +#include <stdbool.h>
> +
> +/**
> + * struct cli_ch_state - state information for reading cmdline characters
> + *
> + * @esc_len: Number of escape characters read so far
> + * @esc_save: Escape characters collected so far
> + * @emit_upto: Next character to emit from esc_save (0 if not emitting)
> + */
> +struct cli_ch_state {
> + int esc_len;
> + char esc_save[8];
> + int emit_upto;
> +};
> +
> /**
> * Go into the command loop
> *
> @@ -154,5 +169,62 @@ void cli_loop(void);
> void cli_init(void);
>
> #define endtick(seconds) (get_ticks() + (uint64_t)(seconds) * get_tbclk())
> +#define CTL_CH(c) ((c) - 'a' + 1)
> +
> +/**
> + * cli_ch_init() - Set up the initial state to process input characters
> + *
> + * @cch: State to set up
> + */
> +void cli_ch_init(struct cli_ch_state *cch);
> +
> +/**
> + * cli_ch_process() - Process an input character
> + *
> + * When @ichar is 0, this function returns any characters from an invalid escape
> + * sequence which are still pending in the buffer
> + *
> + * Otherwise it processes the input character. If it is an escape character,
> + * then an escape sequence is started and the function returns 0. If we are in
> + * the middle of an escape sequence, the character is processed and may result
> + * in returning 0 (if more characters are needed) or a valid character (if
> + * @ichar finishes the sequence).
> + *
> + * If @ichar is a valid character and there is no escape sequence in progress,
> + * then it is returned as is.
> + *
> + * If the Enter key is pressed, '\n' is returned.
> + *
> + * Usage should be like this::
> + *
> + * struct cli_ch_state cch;
> + *
> + * cli_ch_init(cch);
> + * do
> + * {
> + * int ichar, ch;
> + *
> + * ichar = cli_ch_process(cch, 0);
> + * if (!ichar) {
> + * ch = getchar();
> + * ichar = cli_ch_process(cch, ch);
> + * }
> + * (handle the ichar character)
> + * } while (!done)
> + *
> + * If tstc() is used to look for keypresses, this function can be called with
> + * @ichar set to -ETIMEDOUT if there is no character after 5-10ms. This allows
> + * the ambgiuity between the Escape key and the arrow keys (which generate an
> + * escape character followed by other characters) to be resolved.
> + *
> + * @cch: Current state
> + * @ichar: Input character to process, or 0 if none, or -ETIMEDOUT if no
> + * character has been received within a small number of milliseconds (this
> + * cancels any existing escape sequence and allows pressing the Escape key to
> + * work)
> + * Returns: Resulting input character after processing, 0 if none, '\e' if
> + * an existing escape sequence was cancelled
> + */
> +int cli_ch_process(struct cli_ch_state *cch, int ichar);
>
> #endif
More information about the U-Boot
mailing list