diff --git a/sys/shell/shell.c b/sys/shell/shell.c index 6948e3e4a9a0f..7278ecf9de60e 100644 --- a/sys/shell/shell.c +++ b/sys/shell/shell.c @@ -29,6 +29,8 @@ #include #include #include +#include +#include #include "shell.h" #include "shell_commands.h" @@ -51,6 +53,11 @@ static void flush_if_needed(void) fflush(stdout); #endif } +/** + * Code indicating that the line buffer size was exceeded. + * This definition ensures there's no collision with EOF. + */ +#define ERROR_READLINE_EXCEEDED (EOF - 1) static shell_command_handler_t find_handler(const shell_command_t *command_list, char *command) { @@ -227,42 +234,75 @@ static void handle_input_line(const shell_command_t *command_list, char *line) } } +/** + * Read a single line from standard input into a buffer. + * + * In addition to copying characters, this routine echoes the line back to + * stdout and also supports primitive line editing. + * + * If the input line is too long, the input will still be consumed until the end + * to prevent the next line from containing garbage. + * + * @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. + * The minimum buffer size is 1. + * + * @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 ERROR_READLINE_EXCEEDED if the buffer size was exceeded. + */ static int readline(char *buf, size_t size) { - char *line_buf_ptr = buf; + int curr_pos = 0; + bool length_exceeded = false; + + assert((size_t) size > 0); while (1) { - if ((line_buf_ptr - buf) >= ((int) size) - 1) { - return -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. */ + /* 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) { - *line_buf_ptr = '\0'; + if (c == ETX) { + curr_pos = 0; + length_exceeded = false; + } + + buf[curr_pos] = '\0'; #ifndef SHELL_NO_ECHO _putchar('\r'); _putchar('\n'); #endif - /* return 1 if line is empty, 0 otherwise */ - return c == ETX || line_buf_ptr == buf; + return (length_exceeded)? ERROR_READLINE_EXCEEDED : curr_pos; } - /* QEMU uses 0x7f (DEL) as backspace, while 0x08 (BS) is for most terminals */ - else if (c == 0x08 || c == 0x7f) { - if (line_buf_ptr == buf) { - /* The line is empty. */ + + /* 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; } - *--line_buf_ptr = '\0'; + /* after we dropped characters do not 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'); @@ -271,7 +311,13 @@ static int readline(char *buf, size_t size) #endif } else { - *line_buf_ptr++ = c; + /* 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 @@ -298,12 +344,22 @@ void shell_run_once(const shell_command_t *shell_commands, while (1) { int res = readline(line_buf, len); - if (res == EOF) { - break; - } + switch (res) { - if (!res) { - handle_input_line(shell_commands, line_buf); + case EOF: + return; + + case ERROR_READLINE_EXCEEDED: + puts("shell: maximum line length exceeded"); + break; + + case 0: + puts("shell: line is empty"); + break; + + default: + handle_input_line(shell_commands, line_buf); + break; } print_prompt(); diff --git a/tests/shell/main.c b/tests/shell/main.c index 8f2cf7cc342cb..43089dbeaba63 100644 --- a/tests/shell/main.c +++ b/tests/shell/main.c @@ -56,7 +56,17 @@ static int print_echo(int argc, char **argv) return 0; } +static int print_shell_bufsize(int argc, char **argv) +{ + (void) argc; + (void) argv; + printf("%d\n", SHELL_DEFAULT_BUFSIZE); + + return 0; +} + static const shell_command_t shell_commands[] = { + { "bufsize", "Get the shell's buffer size", print_shell_bufsize }, { "start_test", "starts a test", print_teststart }, { "end_test", "ends a test", print_testend }, { "echo", "prints the input command", print_echo }, diff --git a/tests/shell/tests/01-run.py b/tests/shell/tests/01-run.py index a93cc23d63e90..bbe93d78db4ea 100755 --- a/tests/shell/tests/01-run.py +++ b/tests/shell/tests/01-run.py @@ -7,6 +7,7 @@ # directory for more details. import sys +import os from testrunner import run @@ -40,30 +41,88 @@ CONTROL_D = DLE+'\x04' CMDS = ( - (CONTROL_C, ('>')), - ('start_test', ('[TEST_START]')), - ('end_test', ('[TEST_END]')), - ('\n', ('>')), + ('start_test', '[TEST_START]'), + ('', 'shell: line is empty\r\n'), + (CONTROL_C, '>'), + ('\n', '>'), ('123456789012345678901234567890123456789012345678901234567890', - ('shell: command not found: ' - '123456789012345678901234567890123456789012345678901234567890')), - ('unknown_command', ('shell: command not found: unknown_command')), + 'shell: command not found: ' + '123456789012345678901234567890123456789012345678901234567890'), + ('unknown_command', 'shell: command not found: unknown_command'), ('help', EXPECTED_HELP), - ('echo a string', ('\"echo\"\"a\"\"string\"')), + ('echo a string', '\"echo\"\"a\"\"string\"'), ('ps', EXPECTED_PS), - ('garbage1234'+CONTROL_C, ('>')), # test cancelling a line ('help', EXPECTED_HELP), - ('reboot', ('test_shell.')) + ('reboot', 'test_shell.'), + ('end_test', '[TEST_END]'), ) +PROMPT = '> ' + +BOARD = os.environ['BOARD'] + + +def print_error(message): + FAIL = '\033[91m' + ENDC = '\033[0m' + print(FAIL + message + ENDC) + def check_cmd(child, cmd, expected): + child.expect(PROMPT) child.sendline(cmd) for line in expected: child.expect_exact(line) +def check_startup(child): + child.expect('test_shell.\r\n') + + +def check_and_get_bufsize(child): + child.sendline('bufsize') + child.expect('([0-9]+)\r\n') + bufsize = int(child.match[1]) + + return bufsize + + +def check_line_exceeded(child, bufsize): + + if BOARD == 'nrf52dk': + # looks like the nrf52dk runs in to undefined behaviour when sending more + # than 64 bytes over UART + 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') + + +def check_line_canceling(child): + child.expect(PROMPT) + child.sendline('garbage1234' + CONTROL_C) + garbage_expected = 'garbage1234\r\r\n' + garbage_received = child.read(len(garbage_expected)) + + assert garbage_expected == garbage_received + + def testfunc(child): + # avoid sending an extra empty line on native. + if BOARD == 'native': + child.crlf = '\n' + + check_startup(child) + + bufsize = check_and_get_bufsize(child) + + check_line_exceeded(child, bufsize) + + check_line_canceling(child) + # loop other defined commands and expected output for cmd, expected in CMDS: check_cmd(child, cmd, expected)