nconf: respect i-search search pattern boundaries
diff mbox

Message ID 20180612120441.22325-1-dirk@gouders.net
State New
Headers show

Commit Message

Dirk Gouders June 12, 2018, 12:04 p.m. UTC
This patch adds boundary checks for nconf's i-search pattern.

Further, the pattern buffer is always bzero'ed when '/' is
pressed, so the second line in the code below was not needed (and
otherwise wouldn't have worked as expected):

	state->pattern[strlen(state->pattern)] = c;
	state->pattern[strlen(state->pattern)] = '\0';

Finally, the pattern length was reduced to a length that still
seems sufficient but will not fill more than the top line of the
screen, thus eliminating special treatment needs on resizes or normal
exit of i-search.

Signed-off-by: Dirk Gouders
---
 scripts/kconfig/nconf.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

Comments

Randy Dunlap June 12, 2018, 4:19 p.m. UTC | #1
On 06/12/2018 05:04 AM, Dirk Gouders wrote:
> This patch adds boundary checks for nconf's i-search pattern.
> 
> Further, the pattern buffer is always bzero'ed when '/' is
> pressed, so the second line in the code below was not needed (and
> otherwise wouldn't have worked as expected):
> 
> 	state->pattern[strlen(state->pattern)] = c;
> 	state->pattern[strlen(state->pattern)] = '\0';
> 
> Finally, the pattern length was reduced to a length that still
> seems sufficient but will not fill more than the top line of the
> screen, thus eliminating special treatment needs on resizes or normal
> exit of i-search.
> 
> Signed-off-by: Dirk Gouders

S-O-B: line should also have your email address after your name.

> ---
>  scripts/kconfig/nconf.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)

Your patch descsription makes sense.  I tested nconf (make nconfig)
both with and without this patch.  I didn't like the /search results
in either case.  F1 (Help) says:

 ││ Start incremental, case-insensitive search for STRING in menu entries,   ││
 ││     no regex support, STRING is displayed in upper left corner           ││
 ││                             </>STRING                                    ││
 ││     Remove last character   <Backspace>                                  ││
 ││     Jump to next hit        <Down>                                       ││
 ││     Jump to previous hit    <Up>                                         ││
 ││ Exit menu search mode       </>  <Esc>

but I couldn't get <Down> == "Jump to next hit" to work for me at all
(with or without the patch).

thanks,
Dirk Gouders June 12, 2018, 7:01 p.m. UTC | #2
Randy Dunlap <rdunlap@infradead.org> writes:

> On 06/12/2018 05:04 AM, Dirk Gouders wrote:
>> This patch adds boundary checks for nconf's i-search pattern.
>> 
>> Further, the pattern buffer is always bzero'ed when '/' is
>> pressed, so the second line in the code below was not needed (and
>> otherwise wouldn't have worked as expected):
>> 
>> 	state->pattern[strlen(state->pattern)] = c;
>> 	state->pattern[strlen(state->pattern)] = '\0';
>> 
>> Finally, the pattern length was reduced to a length that still
>> seems sufficient but will not fill more than the top line of the
>> screen, thus eliminating special treatment needs on resizes or normal
>> exit of i-search.
>> 
>> Signed-off-by: Dirk Gouders
>
> S-O-B: line should also have your email address after your name.
>
>> ---
>>  scripts/kconfig/nconf.c | 10 ++++++----
>>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> Your patch descsription makes sense.  I tested nconf (make nconfig)
> both with and without this patch.  I didn't like the /search results
> in either case.  F1 (Help) says:
>
>  ││ Start incremental, case-insensitive search for STRING in menu entries,   ││
>  ││     no regex support, STRING is displayed in upper left corner           ││
>  ││                             </>STRING                                    ││
>  ││     Remove last character   <Backspace>                                  ││
>  ││     Jump to next hit        <Down>                                       ││
>  ││     Jump to previous hit    <Up>                                         ││
>  ││ Exit menu search mode       </>  <Esc>
>
> but I couldn't get <Down> == "Jump to next hit" to work for me at all
> (with or without the patch).

Thank you for testing and commenting on the patch.  I will correct the
S-O-B line and also remove one "search" from the subject.

The navigation through matches works here, did use the up/down arrow
keys?  Because of your comment, I also tested page-up/page-down and
noticed that those keys add 'S', 'R' to the search pattern,
respectively...

Dirk
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/scripts/kconfig/nconf.c b/scripts/kconfig/nconf.c
index 97b78445584b..0e224528aaf9 100644
--- a/scripts/kconfig/nconf.c
+++ b/scripts/kconfig/nconf.c
@@ -1007,11 +1007,12 @@  static void adj_match_dir(match_f *match_direction)
 	/* else, do no change.. */
 }
 
+#define PATTERN_LENGTH 64
 struct match_state
 {
 	int in_search;
 	match_f match_direction;
-	char pattern[256];
+	char pattern[PATTERN_LENGTH];
 };
 
 /* Return 0 means I have handled the key. In such a case, ans should hold the
@@ -1035,8 +1036,8 @@  static int do_match(int key, struct match_state *state, int *ans)
 		return 1;
 
 	if (isalnum(c) || isgraph(c) || c == ' ') {
-		state->pattern[strlen(state->pattern)] = c;
-		state->pattern[strlen(state->pattern)] = '\0';
+		if (strlen(state->pattern) + 1 < PATTERN_LENGTH)
+			state->pattern[strlen(state->pattern)] = c;
 		adj_match_dir(&state->match_direction);
 		*ans = get_mext_match(state->pattern,
 				state->match_direction);
@@ -1049,7 +1050,8 @@  static int do_match(int key, struct match_state *state, int *ans)
 		*ans = get_mext_match(state->pattern,
 				state->match_direction);
 	} else if (key == KEY_BACKSPACE || key == 127) {
-		state->pattern[strlen(state->pattern)-1] = '\0';
+		if (state->pattern[0] != '\0')
+			state->pattern[strlen(state->pattern)-1] = '\0';
 		adj_match_dir(&state->match_direction);
 	} else
 		terminate_search = 1;