Skip to content

Commit

Permalink
Merge pull request #13196 from HendrikVE/shell-readline-refactor
Browse files Browse the repository at this point in the history 
sys/shell: refactor readline function
  • Loading branch information
@fjmolinas
fjmolinas authored May 14, 2020
2 parents 82b41bd + bd3eff3 commit 1f9d299
Show file tree
Hide file tree
Showing 3 changed files with 121 additions and 93 deletions.
4 changes: 0 additions & 4 deletions cpu/esp32/Makefile.include
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,6 @@ endif

ESP_SDK_DIR = $(ESP32_SDK_DIR)

# With the '-Os' option, the ESP32 hangs sporadically in 'tests/bench*' if
# interrupts are disabled too early by benchmark tests.
CFLAGS_OPT ?= -O2

# ESP32 specific flashing options
FLASH_CHIP = esp32
FLASH_MODE ?= dout
Expand Down
189 changes: 104 additions & 85 deletions sys/shell/shell.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,32 +39,38 @@
#include "shell_commands.h"

#define ETX '\x03' /** ASCII "End-of-Text", or ctrl-C */
#if !defined(SHELL_NO_ECHO) || !defined(SHELL_NO_PROMPT)
#ifdef MODULE_NEWLIB
/* use local copy of putchar, as it seems to be inlined,
* enlarging code by 50% */
static void _putchar(int c) {
putchar(c);
}
#define BS '\x08' /** ASCII "Backspace" */
#define DEL '\x7f' /** ASCII "Delete" */

#ifdef MODULE_SHELL_COMMANDS
#define MORE_COMMANDS _shell_command_list
#else
#define _putchar putchar
#endif
#endif
#define MORE_COMMANDS
#endif /* MODULE_SHELL_COMMANDS */

static void flush_if_needed(void)
{
#ifdef MODULE_NEWLIB
fflush(stdout);
#endif
}
#define flush_if_needed() fflush(stdout)
#else
#define flush_if_needed()
#endif /* MODULE_NEWLIB */

#ifndef SHELL_NO_ECHO
#define ECHO_ON 1
#else
#define ECHO_ON 0
#endif /* SHELL_NO_ECHO */

#ifndef SHELL_NO_PROMPT
#define PROMPT_ON 1
#else
#define PROMPT_ON 0
#endif /* SHELL_NO_PROMPT */

static shell_command_handler_t find_handler(const shell_command_t *command_list, char *command)
{
const shell_command_t *command_lists[] = {
command_list,
#ifdef MODULE_SHELL_COMMANDS
_shell_command_list,
#endif
MORE_COMMANDS
};

/* iterating over command_lists */
Expand Down Expand Up @@ -95,9 +101,7 @@ static void print_help(const shell_command_t *command_list)

const shell_command_t *command_lists[] = {
command_list,
#ifdef MODULE_SHELL_COMMANDS
_shell_command_list,
#endif
MORE_COMMANDS
};

/* iterating over command_lists */
Expand Down Expand Up @@ -233,6 +237,40 @@ static void handle_input_line(const shell_command_t *command_list, char *line)
}
}

static inline void print_prompt(void)
{
if (PROMPT_ON) {
putchar('>');
putchar(' ');
}

flush_if_needed();
}

static inline void echo_char(char c)
{
if (ECHO_ON) {
putchar(c);
}
}

static inline void white_tape(void)
{
if (ECHO_ON) {
putchar('\b');
putchar(' ');
putchar('\b');
}
}

static inline void new_line(void)
{
if (ECHO_ON) {
putchar('\r');
putchar('\n');
}
}

/**
* @brief Read a single line from standard input into a buffer.
*
Expand All @@ -242,6 +280,10 @@ static void handle_input_line(const shell_command_t *command_list, char *line)
* If the input line is too long, the input will still be consumed until the end
* to prevent the next line from containing garbage.
*
* We allow Unix (\n), DOS (\r\n), and Mac linebreaks (\r).
* QEMU transmits only a single '\r' == 13 on hitting enter ("-serial stdio").
* DOS newlines are handled like hitting enter twice.
*
* @param buf Buffer where the input will be placed.
* @param size Size of the buffer. The maximum line length will be one less
* than size, to accommodate for the null terminator.
Expand All @@ -250,7 +292,7 @@ static void handle_input_line(const shell_command_t *command_list, char *line)
* @return length of the read line, excluding the terminator, if reading was
* successful.
* @return EOF, if the end of the input stream was reached.
* @return ENOBUFS if the buffer size was exceeded.
* @return -ENOBUFS if the buffer size was exceeded.
*/
static int readline(char *buf, size_t size)
{
Expand All @@ -260,82 +302,59 @@ static int readline(char *buf, size_t size)
assert((size_t) size > 0);

while (1) {
/* At the start of the loop, cur_pos should point inside of
* buf. This ensures the terminator can always fit. */
assert((size_t) curr_pos < size);

int c = getchar();
if (c < 0) {
return EOF;
}

/* We allow Unix linebreaks (\n), DOS linebreaks (\r\n), and Mac
* linebreaks (\r). QEMU transmits only a single '\r' == 13 on hitting
* enter ("-serial stdio"). DOS newlines are handled like hitting enter
* twice, but empty lines are ignored. Ctrl-C cancels the current line.
*/
if (c == '\r' || c == '\n' || c == ETX) {
if (c == ETX) {
switch (c) {

case EOF:
return EOF;

case ETX:
/* Ctrl-C cancels the current line. */
curr_pos = 0;
length_exceeded = false;
}

buf[curr_pos] = '\0';
#ifndef SHELL_NO_ECHO
_putchar('\r');
_putchar('\n');
#endif
/* fall-thru */
case '\r':
/* fall-thru */
case '\n':
buf[curr_pos] = '\0';

new_line();

return (length_exceeded) ? -ENOBUFS : curr_pos;

/* check for backspace: */
case BS: /* 0x08 (BS) for most terminals */
/* fall-thru */
case DEL: /* 0x7f (DEL) when using QEMU */
if (curr_pos > 0) {
curr_pos--;
if ((size_t) curr_pos < size) {
buf[curr_pos] = '\0';
length_exceeded = false;
}
white_tape();
}
break;

return (length_exceeded) ? -ENOBUFS : curr_pos;
default:
/* Always consume characters, but do not not always store them */
if ((size_t) curr_pos < size - 1) {
buf[curr_pos++] = c;
}
else {
length_exceeded = true;
}
echo_char(c);
break;
}

/* check for backspace:
* 0x7f (DEL) when using QEMU
* 0x08 (BS) for most terminals */
if (c == 0x08 || c == 0x7f) {
if (curr_pos == 0) {
/* ignore empty line */
continue;
}

/* after we dropped characters don't edit the line, yet keep the
* visual effects */
if (!length_exceeded) {
buf[--curr_pos] = '\0';
}
/* white-tape the character */
#ifndef SHELL_NO_ECHO
_putchar('\b');
_putchar(' ');
_putchar('\b');
#endif
}
else {
/* Always consume characters, but do not not always store them */
if ((size_t) curr_pos < size - 1) {
buf[curr_pos++] = c;
}
else {
length_exceeded = true;
}
#ifndef SHELL_NO_ECHO
_putchar(c);
#endif
}
flush_if_needed();
}
}

static inline void print_prompt(void)
{
#ifndef SHELL_NO_PROMPT
_putchar('>');
_putchar(' ');
#endif

flush_if_needed();
}

void shell_run_once(const shell_command_t *shell_commands,
char *line_buf, int len)
{
Expand Down
21 changes: 17 additions & 4 deletions tests/shell/tests/01-run.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@
'shell: command not found: '
'123456789012345678901234567890123456789012345678901234567890'),
('unknown_command', 'shell: command not found: unknown_command'),
('hello-willy\b\b\b\borld', 'shell: command not found: hello-world'),
('\b\b\b\becho', '\"echo\"'),
('help', EXPECTED_HELP),
('echo a string', '\"echo\"\"a\"\"string\"'),
('ps', EXPECTED_PS),
Expand Down Expand Up @@ -87,7 +89,7 @@ def check_and_get_bufsize(child):
return bufsize


def check_line_exceeded(child, bufsize):
def check_line_exceeded(child, longline):

if BOARD == 'nrf52dk':
# There is an issue with nrf52dk when the Virtual COM port is connected
Expand All @@ -97,8 +99,6 @@ def check_line_exceeded(child, bufsize):
print_error('test case "check_line_exceeded" broken for nrf52dk. SKIP')
return

longline = "_"*bufsize + "verylong"

child.sendline(longline)
child.expect('shell: maximum line length exceeded')

Expand All @@ -112,6 +112,16 @@ def check_line_canceling(child):
assert garbage_expected == garbage_received


def check_erase_long_line(child, longline):
# FIXME: this only works on native, due to #10634 combined with socat
# insisting in line-buffering the terminal.

if BOARD == 'native':
longline_erased = longline + "\b"*len(longline) + "echo"
child.sendline(longline_erased)
child.expect_exact('"echo"')


def testfunc(child):
# avoid sending an extra empty line on native.
if BOARD == 'native':
Expand All @@ -120,11 +130,14 @@ def testfunc(child):
check_startup(child)

bufsize = check_and_get_bufsize(child)
longline = "_"*bufsize + "verylong"

check_line_exceeded(child, bufsize)
check_line_exceeded(child, longline)

check_line_canceling(child)

check_erase_long_line(child, longline)

# loop other defined commands and expected output
for cmd, expected in CMDS:
check_cmd(child, cmd, expected)
Expand Down

0 comments on commit 1f9d299

Please sign in to comment.