All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] kdb: Fix pager search for multi-line strings
@ 2020-09-09 14:17 Daniel Thompson
  2020-09-14 23:13 ` Doug Anderson
  0 siblings, 1 reply; 2+ messages in thread
From: Daniel Thompson @ 2020-09-09 14:17 UTC (permalink / raw)
  To: Douglas Anderson, Jason Wessel
  Cc: Daniel Thompson, kgdb-bugreport, linux-kernel, patches

Currently using forward search doesn't handle multi-line strings correctly.
The search routine replaces line breaks with \0 during the search and, for
regular searches ("help | grep Common\n"), there is code after the line
has been discarded or printed to replace the break character.

However during a pager search ("help\n" followed by "/Common\n") when the
string is matched we will immediately return to normal output and the code
that should restore the \n becomes unreachable. Fix this by restoring the
replaced character when we disable the search mode and update the comment
acordingly.

Fixes: fb6daa7520f9d ("kdb: Provide forward search at more prompt")
Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
---

Notes:
    In the long term the kdb pager code would probably benefit from a
    bigger rewrite since the way it handles newlines is still quirky
    and confusing. However this fix is easy to backport so I decided
    not to hold it back whilst we wait for code that is not yet
    written.

 kernel/debug/kdb/kdb_io.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/kernel/debug/kdb/kdb_io.c b/kernel/debug/kdb/kdb_io.c
index 9d847ab851db..e240c97086e2 100644
--- a/kernel/debug/kdb/kdb_io.c
+++ b/kernel/debug/kdb/kdb_io.c
@@ -706,12 +706,16 @@ int vkdb_printf(enum kdb_msgsrc src, const char *fmt, va_list ap)
 			size_avail = sizeof(kdb_buffer) - len;
 			goto kdb_print_out;
 		}
-		if (kdb_grepping_flag >= KDB_GREPPING_FLAG_SEARCH)
+		if (kdb_grepping_flag >= KDB_GREPPING_FLAG_SEARCH) {
 			/*
 			 * This was a interactive search (using '/' at more
-			 * prompt) and it has completed. Clear the flag.
+			 * prompt) and it has completed. Replace the \0 with
+			 * its original value to ensure multi-line strings
+			 * are handled properly, and return to normal mode.
 			 */
+			*cphold = replaced_byte;
 			kdb_grepping_flag = 0;
+		}
 		/*
 		 * at this point the string is a full line and
 		 * should be printed, up to the null.

base-commit: f75aef392f869018f78cfedf3c320a6b3fcfda6b
--
2.25.4


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] kdb: Fix pager search for multi-line strings
  2020-09-09 14:17 [PATCH] kdb: Fix pager search for multi-line strings Daniel Thompson
@ 2020-09-14 23:13 ` Doug Anderson
  0 siblings, 0 replies; 2+ messages in thread
From: Doug Anderson @ 2020-09-14 23:13 UTC (permalink / raw)
  To: Daniel Thompson; +Cc: Jason Wessel, kgdb-bugreport, LKML, Patch Tracking

Hi,

On Wed, Sep 9, 2020 at 7:18 AM Daniel Thompson
<daniel.thompson@linaro.org> wrote:
>
> Currently using forward search doesn't handle multi-line strings correctly.
> The search routine replaces line breaks with \0 during the search and, for
> regular searches ("help | grep Common\n"), there is code after the line
> has been discarded or printed to replace the break character.
>
> However during a pager search ("help\n" followed by "/Common\n") when the
> string is matched we will immediately return to normal output and the code
> that should restore the \n becomes unreachable. Fix this by restoring the
> replaced character when we disable the search mode and update the comment
> acordingly.
>
> Fixes: fb6daa7520f9d ("kdb: Provide forward search at more prompt")
> Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
> ---
>
> Notes:
>     In the long term the kdb pager code would probably benefit from a
>     bigger rewrite since the way it handles newlines is still quirky
>     and confusing. However this fix is easy to backport so I decided
>     not to hold it back whilst we wait for code that is not yet
>     written.
>
>  kernel/debug/kdb/kdb_io.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)

Wow, more kdb functionality that I never knew anything about!  Seems
like a reasonable fix to me even if I agree that the code could use a
bigger cleanup.

Reviewed-by: Douglas Anderson <dianders@chromium.org>

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2020-09-14 23:14 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-09 14:17 [PATCH] kdb: Fix pager search for multi-line strings Daniel Thompson
2020-09-14 23:13 ` Doug Anderson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.