| Message ID | 20191008132043.7966-5-daniel.thompson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | kdb: Cleanup code to read user input and handle escape sequences | expand |
Hi, On Tue, Oct 8, 2019 at 6:21 AM Daniel Thompson <daniel.thompson@linaro.org> wrote: > > Currently if an escape timer is interrupted by a character from a > different input source then the new character is discarded and the > function returns '\e' (which will be discarded by the level above). > It is hard to see why this would ever be the desired behaviour. I guess the 2nd input source would be if you enable keyboard input? Personally I've never used this myself, but your functional change seems OK to me. > Fix this to return the new character rather then the '\e'. s/then/than > This is a bigger refactor that might be expected because the new > character needs to go through escape sequence detection. > > Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org> > --- > kernel/debug/kdb/kdb_io.c | 37 ++++++++++++++++++------------------- > 1 file changed, 18 insertions(+), 19 deletions(-) > > diff --git a/kernel/debug/kdb/kdb_io.c b/kernel/debug/kdb/kdb_io.c > index a9e73bc9d1c3..288dd1babf90 100644 > --- a/kernel/debug/kdb/kdb_io.c > +++ b/kernel/debug/kdb/kdb_io.c > @@ -122,8 +122,8 @@ static int kdb_getchar(void) > { > #define ESCAPE_UDELAY 1000 > #define ESCAPE_DELAY (2*1000000/ESCAPE_UDELAY) /* 2 seconds worth of udelays */ > - char escape_data[5]; /* longest vt100 escape sequence is 4 bytes */ > - char *ped = escape_data; > + char buf[4]; /* longest vt100 escape sequence is 4 bytes */ > + char *pbuf = buf; > int escape_delay = 0; > get_char_func *f, *f_escape = NULL; > int key; > @@ -145,27 +145,26 @@ static int kdb_getchar(void) > continue; > } > > - if (escape_delay == 0 && key == '\e') { > - escape_delay = ESCAPE_DELAY; > - ped = escape_data; > + /* > + * When the first character is received (or we get a change > + * input source) we set ourselves up to handle an escape > + * sequences (just in case). > + */ > + if (f_escape != f) { > f_escape = f; Would it make sense to rename "f_escape" to "f_last" or "f_prev" now? Essentially this logic now happens every time you change input sources. -Doug
diff --git a/kernel/debug/kdb/kdb_io.c b/kernel/debug/kdb/kdb_io.c index a9e73bc9d1c3..288dd1babf90 100644 --- a/kernel/debug/kdb/kdb_io.c +++ b/kernel/debug/kdb/kdb_io.c @@ -122,8 +122,8 @@ static int kdb_getchar(void) { #define ESCAPE_UDELAY 1000 #define ESCAPE_DELAY (2*1000000/ESCAPE_UDELAY) /* 2 seconds worth of udelays */ - char escape_data[5]; /* longest vt100 escape sequence is 4 bytes */ - char *ped = escape_data; + char buf[4]; /* longest vt100 escape sequence is 4 bytes */ + char *pbuf = buf; int escape_delay = 0; get_char_func *f, *f_escape = NULL; int key; @@ -145,27 +145,26 @@ static int kdb_getchar(void) continue; } - if (escape_delay == 0 && key == '\e') { - escape_delay = ESCAPE_DELAY; - ped = escape_data; + /* + * When the first character is received (or we get a change + * input source) we set ourselves up to handle an escape + * sequences (just in case). + */ + if (f_escape != f) { f_escape = f; - } - if (escape_delay) { - if (f_escape != f) - return '\e'; - - *ped++ = key; - key = kdb_read_handle_escape(escape_data, - ped - escape_data); - if (key < 0) - return '\e'; - if (key == 0) - continue; + pbuf = buf; + escape_delay = ESCAPE_DELAY; } - break; /* A key to process */ + *pbuf++ = key; + key = kdb_read_handle_escape(buf, pbuf - buf); + if (key < 0) /* no escape sequence; return first character */ + return buf[0]; + if (key > 0) + return key; } - return key; + + unreachable(); } /*
Currently if an escape timer is interrupted by a character from a different input source then the new character is discarded and the function returns '\e' (which will be discarded by the level above). It is hard to see why this would ever be the desired behaviour. Fix this to return the new character rather then the '\e'. This is a bigger refactor that might be expected because the new character needs to go through escape sequence detection. Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org> --- kernel/debug/kdb/kdb_io.c | 37 ++++++++++++++++++------------------- 1 file changed, 18 insertions(+), 19 deletions(-) -- 2.21.0