On Mon, Apr 22, 2024 at 04:52:04PM -0700, Doug Anderson wrote: > Hi, > > On Mon, Apr 22, 2024 at 9:37 AM Daniel Thompson > <daniel.thomp...@linaro.org> wrote: > > > > Currently when kdb_read() needs to reposition the cursor it uses copy and > > paste code that works by injecting an '\0' at the cursor position before > > delivering a carriage-return and reprinting the line (which stops at the > > '\0'). > > > > Tidy up the code by hoisting the copy and paste code into an appropriately > > named function. Additionally let's replace the '\0' injection with a > > proper field width parameter so that the string will be abridged during > > formatting instead. > > > > Cc: sta...@vger.kernel.org # Not a bug fix but it is needed for later bug > > fixes > > Signed-off-by: Daniel Thompson <daniel.thomp...@linaro.org> > > --- > > kernel/debug/kdb/kdb_io.c | 34 ++++++++++++++-------------------- > > 1 file changed, 14 insertions(+), 20 deletions(-) > > Looks like a nice fix, but I think this'll create a compile warning on > some compilers. The variable "tmp" is no longer used, I think. > > Once the "tmp" variable is deleted, feel free to add my Reviewed-by.
Good spot. I'll fix that. > NOTE: patch #7 in your series re-adds a user of "tmp", but since this > one is "Cc: stable" you will need to delete it here and then re-add it > in patch #7. > > > > diff --git a/kernel/debug/kdb/kdb_io.c b/kernel/debug/kdb/kdb_io.c > > index 06dfbccb10336..a42607e4d1aba 100644 > > --- a/kernel/debug/kdb/kdb_io.c > > +++ b/kernel/debug/kdb/kdb_io.c > > @@ -184,6 +184,13 @@ char kdb_getchar(void) > > unreachable(); > > } > > > > +static void kdb_position_cursor(char *prompt, char *buffer, char *cp) > > +{ > > + kdb_printf("\r%s", kdb_prompt_str); > > + if (cp > buffer) > > + kdb_printf("%.*s", (int)(cp - buffer), buffer); > > nit: personally, I'd take the "if" statement out. I'm nearly certain > that kdb_printf() can handle zero-length for the width argument and > "buffer" can never be _after_ cp (so you can't get negative). The kernel will correctly format zero-width fields... but kdb_printf() will also inject the empty string into the log if running with LOGGING=1. It is true the dmesg output with LOGGING=1 is pretty nasty when doing line editing but I still didn't want to make it worse! Oh... and we can't combine into one call to kdb_printf() since that renders into a fixed length static buffer and we could get unwanted truncation. I might just add a comment about that. Daniel. _______________________________________________ Kgdb-bugreport mailing list Kgdb-bugreport@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kgdb-bugreport