[v2] nconf: respect i-search pattern boundaries
diff mbox

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

Commit Message

Dirk Gouders June 12, 2018, 7:06 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 <dirk@gouders.net>
---
Changes in v2: Correct S-O-B line, remove double search from subject.
---
 scripts/kconfig/nconf.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

Comments

Randy Dunlap June 13, 2018, 1:09 a.m. UTC | #1
On 06/12/2018 12:06 PM, 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 <dirk@gouders.net>
> ---
> Changes in v2: Correct S-O-B line, remove double search from subject.
> ---
>  scripts/kconfig/nconf.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)

Hi Dirk,

For v1 of the patch:  no, I wasn't using PageUp/PageDown, just the up/down arrow
keys.

Maybe I'm confused about /search in nconfig.  nconfig 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>

First of all, does this only search in the Kconfig prompt strings?

(from top-level menu)
OK, I want to search for "debug", so I enter "/debug" and the upper left
corner displays: searching: debug

The nconfig highlight bar goes to the Device Drivers menu.
That's it.  It hasn't found "debug" (it found "de" AFAICT).
Using Up/Down arrow keys don't do anything.  I gave up.

What am I doing wrong?

thanks,
Dirk Gouders June 13, 2018, 5:18 a.m. UTC | #2
Randy Dunlap <rdunlap@infradead.org> writes:

> On 06/12/2018 12:06 PM, 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 <dirk@gouders.net>
>> ---
>> Changes in v2: Correct S-O-B line, remove double search from subject.
>> ---
>>  scripts/kconfig/nconf.c | 10 ++++++----
>>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> Hi Dirk,
>
> For v1 of the patch:  no, I wasn't using PageUp/PageDown, just the up/down arrow
> keys.
>
> Maybe I'm confused about /search in nconfig.  nconfig 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>
>
> First of all, does this only search in the Kconfig prompt strings?

Hi Randy,

yes, it only searches the items in the current menu and this
probably was the reason to make you think the arrow keys don't work.

This is also the case for the i-search RFC I posted for mconf.  I know,
it would be extremely convenient to i-search in _all_ config symbols from
the top level menu and then navigate to the submenus that contain
matches but such a change needs a lot more work to be done.

Dirk

> (from top-level menu)
> OK, I want to search for "debug", so I enter "/debug" and the upper left
> corner displays: searching: debug
>
> The nconfig highlight bar goes to the Device Drivers menu.
> That's it.  It hasn't found "debug" (it found "de" AFAICT).
> Using Up/Down arrow keys don't do anything.  I gave up.
>
> What am I doing wrong?
>
> thanks,
--
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
Randy Dunlap June 13, 2018, 5:28 a.m. UTC | #3
On 06/12/2018 10:18 PM, Dirk Gouders wrote:
> Randy Dunlap <rdunlap@infradead.org> writes:
> 
>> On 06/12/2018 12:06 PM, 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 <dirk@gouders.net>
>>> ---
>>> Changes in v2: Correct S-O-B line, remove double search from subject.
>>> ---
>>>  scripts/kconfig/nconf.c | 10 ++++++----
>>>  1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> Hi Dirk,
>>
>> For v1 of the patch:  no, I wasn't using PageUp/PageDown, just the up/down arrow
>> keys.
>>
>> Maybe I'm confused about /search in nconfig.  nconfig 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>
>>
>> First of all, does this only search in the Kconfig prompt strings?
> 
> Hi Randy,
> 
> yes, it only searches the items in the current menu and this
> probably was the reason to make you think the arrow keys don't work.
> 
> This is also the case for the i-search RFC I posted for mconf.  I know,
> it would be extremely convenient to i-search in _all_ config symbols from
> the top level menu and then navigate to the submenus that contain
> matches but such a change needs a lot more work to be done.
> 

Ah, I see.  Thanks for explaining.

> 
>> (from top-level menu)
>> OK, I want to search for "debug", so I enter "/debug" and the upper left
>> corner displays: searching: debug
>>
>> The nconfig highlight bar goes to the Device Drivers menu.
>> That's it.  It hasn't found "debug" (it found "de" AFAICT).
>> Using Up/Down arrow keys don't do anything.  I gave up.
>>
>> What am I doing wrong?
>>
>> thanks,

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;