I have a couple minor notes below. More important, does this change require updating documentation?
I know we have a somewhat aging shell-specific guide: https://docs.rtems.org/branches/master/shell/index.html On Fri, Jan 19, 2024 at 5:19 AM <dufa...@hda.com> wrote: > > From: Peter Dufault <dufa...@hda.com> > > - Fix detection of timeout in rtems_shell_term_wait_for(). > > - Switch to using "termios" VTIME inter-character timeout. > The previous implementation is dependent on the BSP clock tick value. > > Updates #4763 > --- > cpukit/libmisc/shell/shell.c | 101 > ++++++++++++++++++++++++++----------------- > 1 file changed, 62 insertions(+), 39 deletions(-) > > diff --git a/cpukit/libmisc/shell/shell.c b/cpukit/libmisc/shell/shell.c > index 9cefc80..60cdb4f 100644 > --- a/cpukit/libmisc/shell/shell.c > +++ b/cpukit/libmisc/shell/shell.c > @@ -805,28 +805,52 @@ void rtems_shell_print_env( > } > #endif > > +/* Debugging output for the terminal I/O associated with window size > detection > + * can be sent with rtems_shell_winsize_db(). > + */ > + > +/* Window size detection knobs. > + */ > +static bool rtems_shell_winsize_dbf; /* Debug. "true" to debug. */ Is this necessary? How does the application set/test this? > +static int rtems_shell_winsize_vtime = 1; /* VTIME timeout in .1 seconds */ > + > +static void rtems_shell_winsize_vdb(const char *format, va_list ap) { > + if (rtems_shell_winsize_dbf == false) > + return; > + printf("shell_winsize: "); > + vprintf(format, ap); > + fflush(stdout); > +} > + > +static void rtems_shell_winsize_db(const char *format, ...) { > + va_list ap; > + va_start(ap, format); > + rtems_shell_winsize_vdb(format, ap); > + va_end(ap); > +} > + > /* > * Wait for the string to return or timeout. > */ > -static bool rtems_shell_term_wait_for(const int fd, const char* str, const > int timeout) > +static bool rtems_shell_term_wait_for(const int fd, const char* str) > { > - int msec = timeout; > int i = 0; > - while (msec-- > 0 && str[i] != '\0') { > + while (str[i] != '\0') { > char ch[2]; > - if (read(fd, &ch[0], 1) == 1) { > - fflush(stdout); > + ssize_t n; > + if ((n = read(fd, &ch[0], 1)) == 1) { > if (ch[0] != str[i++]) { > + rtems_shell_winsize_db("Wrong char at char %d.\n", i); > return false; > } > - msec = timeout; > } else { > - usleep(1000); > + rtems_shell_winsize_db("%s reading char %d.\n", > + (n == 0) ? "Timeout" : "Error", i); I'd suggest putting the 'i' on it's own line here. I don't know that there's a specific style guide for this file, but having trailing statements after the ternary is a bit harder to read. imo > + return false; > } > } > - if (msec == 0) { > - return false; > - } > + > + rtems_shell_winsize_db("Matched string.\n"); > return true; > } > > @@ -836,41 +860,42 @@ static bool rtems_shell_term_wait_for(const int fd, > const char* str, const int t > static int rtems_shell_term_buffer_until(const int fd, > char* buf, > const int size, > - const char* end, > - const int timeout) > + const char* end) > { > - int msec = timeout; > int i = 0; > int e = 0; > memset(&buf[0], 0, size); > - while (msec-- > 0 && i < size && end[e] != '\0') { > + while (i < size && end[e] != '\0') { > char ch[2]; > - if (read(fd, &ch[0], 1) == 1) { > - fflush(stdout); > + ssize_t n; > + if ( (n = read(fd, &ch[0], 1)) == 1) { > buf[i++] = ch[0]; > if (ch[0] == end[e]) { > e++; > } else { > + rtems_shell_winsize_db("Reset search for end string.\n"); > e = 0; > } > - msec = timeout; > } else { > - usleep(1000); > + /* Error or timeout. */ > + rtems_shell_winsize_db( > + "%s looking for end string \"%s\". Read \"%s\".\n", > + (n == 0) ? "Timeout" : "Error", end, buf > + ); > + return -1; > } > } > - if (msec == 0 || end[e] != '\0') { > - return -1; > - } > - i -= e; > + i -= e; /* Toss away the matched end. */ > if (i < size) { > buf[i] = '\0'; > } > + rtems_shell_winsize_db("Found end string \"%s\".\n", end); > return i; > } > > /* > - * Determine if the terminal has the row and column values > - * swapped > + * Determine if this is a version of the "tmux" terminal emulator with > + * the row and column values swapped > * > * https://github.com/tmux/tmux/issues/3457 > * > @@ -878,19 +903,19 @@ static int rtems_shell_term_buffer_until(const int fd, > * in the time it takes to get the fix into code so see if tmux is > * running and which version and work around the bug. > * > - * The terminal device needs to have VMIN=0, and VTIME=0 > + * The terminal device needs to have VMIN=0, and VTIME set to the > + * desired timeout in .1 seconds. > */ > -static bool rtems_shell_term_row_column_swapped(const int fd, const int > timeout) { > +static bool rtems_shell_term_row_column_swapped(const int fd, const int > fout) { > char buf[64]; > memset(&buf[0], 0, sizeof(buf)); > /* > * CSI > Ps q > * Ps = 0 => DCS > | text ST > */ > - fputs("\033[>0q", stdout); > - fflush(stdout); > - if (rtems_shell_term_wait_for(fd, "\033P>|", timeout)) { > - int len = rtems_shell_term_buffer_until(fd, buf, sizeof(buf), "\033\\", > timeout); > + write(fout, "\033[>0q", 5); > + if (rtems_shell_term_wait_for(fd, "\033P>|")) { > + int len = rtems_shell_term_buffer_until(fd, buf, sizeof(buf), "\033\\"); > if (len > 0) { > if (memcmp(buf, "tmux ", 5) == 0) { > static const char* bad_versions[] = { > @@ -916,8 +941,8 @@ static bool rtems_shell_term_row_column_swapped(const int > fd, const int timeout) > static void rtems_shell_winsize( void ) > { > const int fd = fileno(stdin); > + const int fout = fileno(stdout); > struct winsize ws; > - const int timeout = 150; > char buf[64]; > bool ok = false; > int lines = 0; > @@ -933,18 +958,16 @@ static void rtems_shell_winsize( void ) > if (tcgetattr(fd, &cterm) >= 0) { > struct termios term = cterm; > term.c_cc[VMIN] = 0; > - term.c_cc[VTIME] = 0; > - if (tcsetattr (fd, TCSADRAIN, &term) >= 0) { > + term.c_cc[VTIME] = rtems_shell_winsize_vtime; > + if (tcsetattr (fd, TCSAFLUSH, &term) >= 0) { > memset(&buf[0], 0, sizeof(buf)); > /* > * > https://invisible-island.net/xterm/ctlseqs/ctlseqs.html#h3-Miscellaneous > * > * CSI 1 8 t > */ > - fputs("\033[18t", stdout); > - fflush(stdout); > - if (rtems_shell_term_wait_for(fd, "\033[8;", timeout)) { > - int len = rtems_shell_term_buffer_until(fd, buf, sizeof(buf), ";", > timeout); > + if (write(fout, "\033[18t", 5) == 5 && rtems_shell_term_wait_for(fd, > "\033[8;")) { > + int len = rtems_shell_term_buffer_until(fd, buf, sizeof(buf), ";"); > if (len > 0) { > int i; > lines = 0; > @@ -953,7 +976,7 @@ static void rtems_shell_winsize( void ) > lines *= 10; > lines += buf[i++] - '0'; > } > - len = rtems_shell_term_buffer_until(fd, buf, sizeof(buf), "t", > timeout); > + len = rtems_shell_term_buffer_until(fd, buf, sizeof(buf), "t"); > if (len > 0) { > cols = 0; > i = 0; > @@ -966,7 +989,7 @@ static void rtems_shell_winsize( void ) > } > } > } > - if (rtems_shell_term_row_column_swapped(fd, timeout)) { > + if (ok && rtems_shell_term_row_column_swapped(fd, fout)) { > int tmp = lines; > lines = cols; > cols = tmp; > -- > 1.8.3.1 > > _______________________________________________ > devel mailing list > devel@rtems.org > http://lists.rtems.org/mailman/listinfo/devel _______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel