Messages in this thread | | | From | Doug Anderson <> | Date | Tue, 8 Oct 2019 15:21:15 -0700 | Subject | Re: [PATCH v2 4/5] kdb: Improve handling of characters from different input sources |
| |
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
| |