* [RFC] nconf bug fixes and improvements
@ 2011-08-29 9:09 Cheng Renquan
2011-08-29 14:14 ` Arnaud Lacombe
` (4 more replies)
0 siblings, 5 replies; 35+ messages in thread
From: Cheng Renquan @ 2011-08-29 9:09 UTC (permalink / raw)
To: Michal Marek, linux-kbuild; +Cc: linux-kernel, Nir Tzachar, crquan
bug fixes:
1) char dialog_input_result[256]; is not enough for config item like:
CONFIG_EXTRA_FIRMWARE="iwlwifi-100-5.ucode iwlwifi-1000-3.ucode
iwlwifi-3945-2.ucode iwlwifi-4965-2.ucode iwlwifi-5000-1.ucode
iwlwifi-5000-2.ucode iwlwifi-5150-2.ucode iwlwifi-6000-4.ucode
iwlwifi-6000g2a-5.ucode iwlwifi-6000g2b-5.ucode iwlwifi-6050-4.ucode
iwlwifi-6050-5.ucode radeon/CEDAR_me.bin radeon/CEDAR_pfp.bin
radeon/CEDAR_rlc.bin radeon/CYPRESS_me.bin radeon/CYPRESS_pfp.bin
radeon/CYPRESS_rlc.bin radeon/JUNIPER_me.bin radeon/JUNIPER_pfp.bin
radeon/JUNIPER_rlc.bin radeon/R600_rlc.bin radeon/R700_rlc.bin
radeon/REDWOOD_me.bin radeon/REDWOOD_pfp.bin radeon/REDWOOD_rlc.bin"
the original nconf just stack overflow / crashed when dealing with
longer than 256 bytes strings; Since the original menuconfig also just
uses a fixed length buffer [MAX_LEN=2048] which works for the years,
here I just append a 0 make it work in the easiest way; if required,
it could also be changed to a dynamically allocated buffer;
char dialog_input_result[MAX_LEN + 1];
2) memmove's 3rd argument should be len-cursor_position+1, the
original len+1 may cause segment fault in theory;
memmove(&result[cursor_position+1],
&result[cursor_position],
- len+1);
+ len-cursor_position+1);
3) typo:
- mvprintw(0, 0, "unknow key: %d\n", res);
+ mvprintw(0, 0, "unknown key: %d\n", res);
improvement:
1) its original conf_string doesn't work with longer string values
(longer than its dialog box width), not at all
(or may work in an invisible way if anyone has tried that)
Here I added a new variable cursor_form_win to record that new state:
cursor of the input box; and make it fun:
when you move cursor to almost left/right edge, it auto adjust text
to center, by half prompt box width;
2) Remove KEY_UP as RIGHT and KEY_DOWN as LEFT,
Add Home/End to locate the string begin/end;
Emacs-like key bind (C-a/C-e as Home/End, C-f/C-b as forward/backward);
this keybind I'd like but may be controversial, it could be
discussed and separated;
This is just [Request for Comments], it just works here on one of my
distributor kernels,
if anyone may think it's useful, please feedback and I would like to
split it into
patch series and rebase to linus latest branch;
Thanks,
--- /mnt/static/usr/src/linux-3.0-gentoo-r1/scripts/kconfig/nconf.c 2011-07-21
19:17:23.000000000 -0700
+++ /mnt/dynamic/usr/src/linux-3.0-gentoo-r1/scripts/kconfig/nconf.c 2011-08-28
18:08:05.699883340 -0700
@@ -1360,7 +1360,7 @@ static void conf_choice(struct menu *men
static void conf_string(struct menu *menu)
{
const char *prompt = menu_get_prompt(menu);
- char dialog_input_result[256];
+ char dialog_input_result[2560];
while (1) {
int res;
--- /mnt/static/usr/src/linux-3.0-gentoo-r1/scripts/kconfig/nconf.gui.c 2011-07-21
19:17:23.000000000 -0700
+++ /mnt/dynamic/usr/src/linux-3.0-gentoo-r1/scripts/kconfig/nconf.gui.c 2011-08-28
18:00:56.441862565 -0700
@@ -367,7 +367,15 @@ int dialog_inputbox(WINDOW *main_window,
int i, x, y;
int res = -1;
int cursor_position = strlen(init);
+ int cursor_form_win;
+ if (strlen(init) +80 > result_len) {
+ (void) attrset(attributes[FUNCTION_HIGHLIGHT] | A_BLINK);
+ mvprintw(0, 0, "result_len(%d) not enough to contain
init_strlen(%d) in %s:%d:%s\n",
+ result_len, strlen(init),
+ __FILE__, __LINE__, __func__);
+ flash();
+ }
/* find the widest line of msg: */
prompt_lines = get_line_no(prompt);
@@ -405,7 +413,11 @@ int dialog_inputbox(WINDOW *main_window,
fill_window(prompt_win, prompt);
mvwprintw(form_win, 0, 0, "%*s", prompt_width, " ");
- mvwprintw(form_win, 0, 0, "%s", result);
+ cursor_form_win = min(cursor_position, prompt_width-1);
+ mvwprintw(form_win, 0, 0, "%s",
+ result + cursor_position-cursor_form_win);
+ mvprintw(0, 0, "cursor_position=%d, cursor_form_win=%d, prompt_width=%d\n",
+ cursor_position, cursor_form_win, prompt_width);
/* create panels */
panel = new_panel(win);
@@ -416,6 +428,8 @@ int dialog_inputbox(WINDOW *main_window,
touchwin(win);
refresh_all_windows(main_window);
while ((res = wgetch(form_win))) {
+ mvprintw(0, 0, "got key: %d\n", res);
+
int len = strlen(result);
switch (res) {
case 10: /* ENTER */
@@ -431,6 +445,13 @@ int dialog_inputbox(WINDOW *main_window,
&result[cursor_position],
len-cursor_position+1);
cursor_position--;
+ if (cursor_form_win < 5
+ && cursor_position > 5)
+ cursor_form_win += prompt_width/2;
+ else
+ cursor_form_win--;
+ if (cursor_form_win > cursor_position)
+ cursor_form_win = cursor_position;
}
break;
case KEY_DC:
@@ -440,16 +461,41 @@ int dialog_inputbox(WINDOW *main_window,
len-cursor_position+1);
}
break;
- case KEY_UP:
+ case 6: /* Ctrl-F */
case KEY_RIGHT:
- if (cursor_position < len &&
- cursor_position < min(result_len, prompt_width))
+ if (cursor_position < len) {
cursor_position++;
+ if (cursor_form_win > prompt_width-5
+ && cursor_position < len-5)
+ cursor_form_win -= prompt_width/2;
+ else
+ cursor_form_win++;
+ if (cursor_form_win < min(cursor_position, prompt_width-1) -
(len-cursor_position))
+ cursor_form_win = min(cursor_position, prompt_width-1) -
(len-cursor_position);
+ }
break;
- case KEY_DOWN:
+ case 2: /* Ctrl-B */
case KEY_LEFT:
- if (cursor_position > 0)
+ if (cursor_position > 0) {
cursor_position--;
+ if (cursor_form_win < 5
+ && cursor_position > 5)
+ cursor_form_win += prompt_width/2;
+ else
+ cursor_form_win--;
+ if (cursor_form_win > cursor_position)
+ cursor_form_win = cursor_position;
+ }
+ break;
+ case 1: /* Ctrl-A */
+ case KEY_HOME:
+ cursor_position = 0;
+ cursor_form_win = 0;
+ break;
+ case 5: /* Ctrl-E */
+ case KEY_END:
+ cursor_position = len;
+ cursor_form_win = min(cursor_position, prompt_width-1);
break;
default:
if ((isgraph(res) || isspace(res)) &&
@@ -457,19 +503,24 @@ int dialog_inputbox(WINDOW *main_window,
/* insert the char at the proper position */
memmove(&result[cursor_position+1],
&result[cursor_position],
- len+1);
+ len-cursor_position+1);
result[cursor_position] = res;
cursor_position++;
+ if (cursor_form_win < prompt_width-1)
+ cursor_form_win++;
} else {
- mvprintw(0, 0, "unknow key: %d\n", res);
+ mvprintw(0, 0, "unknown key: %d\n", res);
}
break;
}
+ mvprintw(0, 0, "got key: %d, cursor_position=%d, cursor_form_win=%d\n",
+ res, cursor_position, cursor_form_win);
wmove(form_win, 0, 0);
wclrtoeol(form_win);
mvwprintw(form_win, 0, 0, "%*s", prompt_width, " ");
- mvwprintw(form_win, 0, 0, "%s", result);
- wmove(form_win, 0, cursor_position);
+ mvwprintw(form_win, 0, 0, "%s",
+ result + cursor_position-cursor_form_win);
+ wmove(form_win, 0, cursor_form_win);
touchwin(win);
refresh_all_windows(main_window);
--
Cheng Renquan (程任全)
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [RFC] nconf bug fixes and improvements
2011-08-29 9:09 [RFC] nconf bug fixes and improvements Cheng Renquan
@ 2011-08-29 14:14 ` Arnaud Lacombe
2011-08-29 16:53 ` Sam Ravnborg
` (3 subsequent siblings)
4 siblings, 0 replies; 35+ messages in thread
From: Arnaud Lacombe @ 2011-08-29 14:14 UTC (permalink / raw)
To: Cheng Renquan; +Cc: Michal Marek, linux-kbuild, linux-kernel, Nir Tzachar
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=UTF-8, Size: 11968 bytes --]
Hi,
On Mon, Aug 29, 2011 at 5:09 AM, Cheng Renquan <crquan@gmail.com> wrote:
> bug fixes:
>
Please split diff in logical changes, one patch per issue. This is
unreviewable in the current state. And please, use git.
See `Documentation/SubmittingPatches' for further informations.
Thanks,
- Arnaud
> 1) char dialog_input_result[256]; is not enough for config item like:
> Â CONFIG_EXTRA_FIRMWARE="iwlwifi-100-5.ucode iwlwifi-1000-3.ucode
> iwlwifi-3945-2.ucode iwlwifi-4965-2.ucode iwlwifi-5000-1.ucode
> iwlwifi-5000-2.ucode iwlwifi-5150-2.ucode iwlwifi-6000-4.ucode
> iwlwifi-6000g2a-5.ucode iwlwifi-6000g2b-5.ucode iwlwifi-6050-4.ucode
> iwlwifi-6050-5.ucode radeon/CEDAR_me.bin radeon/CEDAR_pfp.bin
> radeon/CEDAR_rlc.bin radeon/CYPRESS_me.bin radeon/CYPRESS_pfp.bin
> radeon/CYPRESS_rlc.bin radeon/JUNIPER_me.bin radeon/JUNIPER_pfp.bin
> radeon/JUNIPER_rlc.bin radeon/R600_rlc.bin radeon/R700_rlc.bin
> radeon/REDWOOD_me.bin radeon/REDWOOD_pfp.bin radeon/REDWOOD_rlc.bin"
>
> Â the original nconf just stack overflow / crashed when dealing with
> longer than 256 bytes strings; Since the original menuconfig also just
> uses a fixed length buffer [MAX_LEN=2048] which works for the years,
> here I just append a 0 make it work in the easiest way; if required,
> it could also be changed to a dynamically allocated buffer;
>
> Â char dialog_input_result[MAX_LEN + 1];
>
> 2) memmove's 3rd argument should be len-cursor_position+1, the
> original len+1 may cause segment fault in theory;
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â memmove(&result[cursor_position+1],
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â &result[cursor_position],
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â len+1);
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â len-cursor_position+1);
>
> 3) typo:
>
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â mvprintw(0, 0, "unknow key: %d\n", res);
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â mvprintw(0, 0, "unknown key: %d\n", res);
>
>
> improvement:
> 1) its original conf_string doesn't work with longer string values
> (longer than its dialog box width), not at all
> Â (or may work in an invisible way if anyone has tried that)
> Â Here I added a new variable cursor_form_win to record that new state:
> Â cursor of the input box; and make it fun:
> Â when you move cursor to almost left/right edge, it auto adjust text
> to center, by half prompt box width;
>
> 2) Remove KEY_UP as RIGHT and KEY_DOWN as LEFT,
> Â Add Home/End to locate the string begin/end;
> Â Emacs-like key bind (C-a/C-e as Home/End, C-f/C-b as forward/backward);
> Â this keybind I'd like but may be controversial, it could be
> discussed and separated;
>
> This is just [Request for Comments], it just works here on one of my
> distributor kernels,
> if anyone may think it's useful, please feedback and I would like to
> split it into
> patch series and rebase to linus latest branch;
>
> Thanks,
>
> --- /mnt/static/usr/src/linux-3.0-gentoo-r1/scripts/kconfig/nconf.c   2011-07-21
> 19:17:23.000000000 -0700
> +++ /mnt/dynamic/usr/src/linux-3.0-gentoo-r1/scripts/kconfig/nconf.c   2011-08-28
> 18:08:05.699883340 -0700
> @@ -1360,7 +1360,7 @@ static void conf_choice(struct menu *men
> Â static void conf_string(struct menu *menu)
> Â {
> Â Â Â Â const char *prompt = menu_get_prompt(menu);
> - Â Â Â char dialog_input_result[256];
> + Â Â Â char dialog_input_result[2560];
>
> Â Â Â Â while (1) {
> Â Â Â Â Â Â Â Â int res;
> --- /mnt/static/usr/src/linux-3.0-gentoo-r1/scripts/kconfig/nconf.gui.c 2011-07-21
> 19:17:23.000000000 -0700
> +++ /mnt/dynamic/usr/src/linux-3.0-gentoo-r1/scripts/kconfig/nconf.gui.c     2011-08-28
> 18:00:56.441862565 -0700
> @@ -367,7 +367,15 @@ int dialog_inputbox(WINDOW *main_window,
> Â Â Â Â int i, x, y;
> Â Â Â Â int res = -1;
> Â Â Â Â int cursor_position = strlen(init);
> + Â Â Â int cursor_form_win;
>
> + Â Â Â if (strlen(init) +80 > result_len) {
> + Â Â Â Â Â Â Â (void) attrset(attributes[FUNCTION_HIGHLIGHT] | A_BLINK);
> + Â Â Â Â Â Â Â mvprintw(0, 0, "result_len(%d) not enough to contain
> init_strlen(%d) in %s:%d:%s\n",
> + Â Â Â Â Â Â Â Â Â Â Â Â result_len, strlen(init),
> + Â Â Â Â Â Â Â Â Â Â Â Â __FILE__, __LINE__, __func__);
> + Â Â Â Â Â Â Â flash();
> + Â Â Â }
>
> Â Â Â Â /* find the widest line of msg: */
> Â Â Â Â prompt_lines = get_line_no(prompt);
> @@ -405,7 +413,11 @@ int dialog_inputbox(WINDOW *main_window,
> Â Â Â Â fill_window(prompt_win, prompt);
>
> Â Â Â Â mvwprintw(form_win, 0, 0, "%*s", prompt_width, " ");
> - Â Â Â mvwprintw(form_win, 0, 0, "%s", result);
> + Â Â Â cursor_form_win = min(cursor_position, prompt_width-1);
> + Â Â Â mvwprintw(form_win, 0, 0, "%s",
> + Â Â Â Â Â Â Â Â result + cursor_position-cursor_form_win);
> + Â Â Â mvprintw(0, 0, "cursor_position=%d, cursor_form_win=%d, prompt_width=%d\n",
> + Â Â Â Â Â Â Â Â cursor_position, cursor_form_win, prompt_width);
>
> Â Â Â Â /* create panels */
> Â Â Â Â panel = new_panel(win);
> @@ -416,6 +428,8 @@ int dialog_inputbox(WINDOW *main_window,
> Â Â Â Â touchwin(win);
> Â Â Â Â refresh_all_windows(main_window);
> Â Â Â Â while ((res = wgetch(form_win))) {
> + Â Â Â Â Â Â Â mvprintw(0, 0, "got key: %d\n", res);
> +
> Â Â Â Â Â Â Â Â int len = strlen(result);
> Â Â Â Â Â Â Â Â switch (res) {
> Â Â Â Â Â Â Â Â case 10: /* ENTER */
> @@ -431,6 +445,13 @@ int dialog_inputbox(WINDOW *main_window,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â &result[cursor_position],
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â len-cursor_position+1);
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â cursor_position--;
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (cursor_form_win < 5
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â && cursor_position > 5)
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â cursor_form_win += prompt_width/2;
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â else
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â cursor_form_win--;
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (cursor_form_win > cursor_position)
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â cursor_form_win = cursor_position;
> Â Â Â Â Â Â Â Â Â Â Â Â }
> Â Â Â Â Â Â Â Â Â Â Â Â break;
> Â Â Â Â Â Â Â Â case KEY_DC:
> @@ -440,16 +461,41 @@ int dialog_inputbox(WINDOW *main_window,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â len-cursor_position+1);
> Â Â Â Â Â Â Â Â Â Â Â Â }
> Â Â Â Â Â Â Â Â Â Â Â Â break;
> - Â Â Â Â Â Â Â case KEY_UP:
> + Â Â Â Â Â Â Â case 6: /* Ctrl-F */
> Â Â Â Â Â Â Â Â case KEY_RIGHT:
> - Â Â Â Â Â Â Â Â Â Â Â if (cursor_position < len &&
> - Â Â Â Â Â Â Â Â Â Â Â Â Â cursor_position < min(result_len, prompt_width))
> + Â Â Â Â Â Â Â Â Â Â Â if (cursor_position < len) {
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â cursor_position++;
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (cursor_form_win > prompt_width-5
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â && cursor_position < len-5)
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â cursor_form_win -= prompt_width/2;
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â else
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â cursor_form_win++;
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (cursor_form_win < min(cursor_position, prompt_width-1) -
> (len-cursor_position))
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â cursor_form_win = min(cursor_position, prompt_width-1) -
> (len-cursor_position);
> + Â Â Â Â Â Â Â Â Â Â Â }
> Â Â Â Â Â Â Â Â Â Â Â Â break;
> - Â Â Â Â Â Â Â case KEY_DOWN:
> + Â Â Â Â Â Â Â case 2: /* Ctrl-B */
> Â Â Â Â Â Â Â Â case KEY_LEFT:
> - Â Â Â Â Â Â Â Â Â Â Â if (cursor_position > 0)
> + Â Â Â Â Â Â Â Â Â Â Â if (cursor_position > 0) {
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â cursor_position--;
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (cursor_form_win < 5
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â && cursor_position > 5)
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â cursor_form_win += prompt_width/2;
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â else
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â cursor_form_win--;
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (cursor_form_win > cursor_position)
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â cursor_form_win = cursor_position;
> + Â Â Â Â Â Â Â Â Â Â Â }
> + Â Â Â Â Â Â Â Â Â Â Â break;
> + Â Â Â Â Â Â Â case 1: /* Ctrl-A */
> + Â Â Â Â Â Â Â case KEY_HOME:
> + Â Â Â Â Â Â Â Â Â Â Â cursor_position = 0;
> + Â Â Â Â Â Â Â Â Â Â Â cursor_form_win = 0;
> + Â Â Â Â Â Â Â Â Â Â Â break;
> + Â Â Â Â Â Â Â case 5: /* Ctrl-E */
> + Â Â Â Â Â Â Â case KEY_END:
> + Â Â Â Â Â Â Â Â Â Â Â cursor_position = len;
> + Â Â Â Â Â Â Â Â Â Â Â cursor_form_win = min(cursor_position, prompt_width-1);
> Â Â Â Â Â Â Â Â Â Â Â Â break;
> Â Â Â Â Â Â Â Â default:
> Â Â Â Â Â Â Â Â Â Â Â Â if ((isgraph(res) || isspace(res)) &&
> @@ -457,19 +503,24 @@ int dialog_inputbox(WINDOW *main_window,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â /* insert the char at the proper position */
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â memmove(&result[cursor_position+1],
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â &result[cursor_position],
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â len+1);
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â len-cursor_position+1);
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â result[cursor_position] = res;
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â cursor_position++;
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (cursor_form_win < prompt_width-1)
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â cursor_form_win++;
> Â Â Â Â Â Â Â Â Â Â Â Â } else {
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â mvprintw(0, 0, "unknow key: %d\n", res);
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â mvprintw(0, 0, "unknown key: %d\n", res);
> Â Â Â Â Â Â Â Â Â Â Â Â }
> Â Â Â Â Â Â Â Â Â Â Â Â break;
> Â Â Â Â Â Â Â Â }
> + Â Â Â Â Â Â Â mvprintw(0, 0, "got key: %d, cursor_position=%d, cursor_form_win=%d\n",
> + Â Â Â Â Â Â Â Â Â Â Â Â res, cursor_position, cursor_form_win);
> Â Â Â Â Â Â Â Â wmove(form_win, 0, 0);
> Â Â Â Â Â Â Â Â wclrtoeol(form_win);
> Â Â Â Â Â Â Â Â mvwprintw(form_win, 0, 0, "%*s", prompt_width, " ");
> - Â Â Â Â Â Â Â mvwprintw(form_win, 0, 0, "%s", result);
> - Â Â Â Â Â Â Â wmove(form_win, 0, cursor_position);
> + Â Â Â Â Â Â Â mvwprintw(form_win, 0, 0, "%s",
> + Â Â Â Â Â Â Â Â Â Â Â Â result + cursor_position-cursor_form_win);
> + Â Â Â Â Â Â Â wmove(form_win, 0, cursor_form_win);
> Â Â Â Â Â Â Â Â touchwin(win);
> Â Â Â Â Â Â Â Â refresh_all_windows(main_window);
>
>
> --
> Cheng Renquan (ç¨ä»»å
¨)
> --
> 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
>
ÿôèº{.nÇ+·®+%Ëÿ±éݶ\x17¥wÿº{.nÇ+·¥{±þG«éÿ{ayº\x1dÊÚë,j\a¢f£¢·hïêÿêçz_è®\x03(éÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?¨èÚ&£ø§~á¶iOæ¬z·vØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?I¥
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [RFC] nconf bug fixes and improvements
@ 2011-08-29 14:14 ` Arnaud Lacombe
0 siblings, 0 replies; 35+ messages in thread
From: Arnaud Lacombe @ 2011-08-29 14:14 UTC (permalink / raw)
To: Cheng Renquan; +Cc: Michal Marek, linux-kbuild, linux-kernel, Nir Tzachar
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=UTF-8, Size: 11935 bytes --]
Hi,
On Mon, Aug 29, 2011 at 5:09 AM, Cheng Renquan <crquan@gmail.com> wrote:
> bug fixes:
>
Please split diff in logical changes, one patch per issue. This is
unreviewable in the current state. And please, use git.
See `Documentation/SubmittingPatches' for further informations.
Thanks,
- Arnaud
> 1) char dialog_input_result[256]; is not enough for config item like:
> Â CONFIG_EXTRA_FIRMWARE="iwlwifi-100-5.ucode iwlwifi-1000-3.ucode
> iwlwifi-3945-2.ucode iwlwifi-4965-2.ucode iwlwifi-5000-1.ucode
> iwlwifi-5000-2.ucode iwlwifi-5150-2.ucode iwlwifi-6000-4.ucode
> iwlwifi-6000g2a-5.ucode iwlwifi-6000g2b-5.ucode iwlwifi-6050-4.ucode
> iwlwifi-6050-5.ucode radeon/CEDAR_me.bin radeon/CEDAR_pfp.bin
> radeon/CEDAR_rlc.bin radeon/CYPRESS_me.bin radeon/CYPRESS_pfp.bin
> radeon/CYPRESS_rlc.bin radeon/JUNIPER_me.bin radeon/JUNIPER_pfp.bin
> radeon/JUNIPER_rlc.bin radeon/R600_rlc.bin radeon/R700_rlc.bin
> radeon/REDWOOD_me.bin radeon/REDWOOD_pfp.bin radeon/REDWOOD_rlc.bin"
>
> Â the original nconf just stack overflow / crashed when dealing with
> longer than 256 bytes strings; Since the original menuconfig also just
> uses a fixed length buffer [MAX_LEN=2048] which works for the years,
> here I just append a 0 make it work in the easiest way; if required,
> it could also be changed to a dynamically allocated buffer;
>
> Â char dialog_input_result[MAX_LEN + 1];
>
> 2) memmove's 3rd argument should be len-cursor_position+1, the
> original len+1 may cause segment fault in theory;
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â memmove(&result[cursor_position+1],
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â &result[cursor_position],
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â len+1);
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â len-cursor_position+1);
>
> 3) typo:
>
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â mvprintw(0, 0, "unknow key: %d\n", res);
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â mvprintw(0, 0, "unknown key: %d\n", res);
>
>
> improvement:
> 1) its original conf_string doesn't work with longer string values
> (longer than its dialog box width), not at all
> Â (or may work in an invisible way if anyone has tried that)
> Â Here I added a new variable cursor_form_win to record that new state:
> Â cursor of the input box; and make it fun:
> Â when you move cursor to almost left/right edge, it auto adjust text
> to center, by half prompt box width;
>
> 2) Remove KEY_UP as RIGHT and KEY_DOWN as LEFT,
> Â Add Home/End to locate the string begin/end;
> Â Emacs-like key bind (C-a/C-e as Home/End, C-f/C-b as forward/backward);
> Â this keybind I'd like but may be controversial, it could be
> discussed and separated;
>
> This is just [Request for Comments], it just works here on one of my
> distributor kernels,
> if anyone may think it's useful, please feedback and I would like to
> split it into
> patch series and rebase to linus latest branch;
>
> Thanks,
>
> --- /mnt/static/usr/src/linux-3.0-gentoo-r1/scripts/kconfig/nconf.c   2011-07-21
> 19:17:23.000000000 -0700
> +++ /mnt/dynamic/usr/src/linux-3.0-gentoo-r1/scripts/kconfig/nconf.c   2011-08-28
> 18:08:05.699883340 -0700
> @@ -1360,7 +1360,7 @@ static void conf_choice(struct menu *men
> Â static void conf_string(struct menu *menu)
> Â {
> Â Â Â Â const char *prompt = menu_get_prompt(menu);
> - Â Â Â char dialog_input_result[256];
> + Â Â Â char dialog_input_result[2560];
>
> Â Â Â Â while (1) {
> Â Â Â Â Â Â Â Â int res;
> --- /mnt/static/usr/src/linux-3.0-gentoo-r1/scripts/kconfig/nconf.gui.c 2011-07-21
> 19:17:23.000000000 -0700
> +++ /mnt/dynamic/usr/src/linux-3.0-gentoo-r1/scripts/kconfig/nconf.gui.c     2011-08-28
> 18:00:56.441862565 -0700
> @@ -367,7 +367,15 @@ int dialog_inputbox(WINDOW *main_window,
> Â Â Â Â int i, x, y;
> Â Â Â Â int res = -1;
> Â Â Â Â int cursor_position = strlen(init);
> + Â Â Â int cursor_form_win;
>
> + Â Â Â if (strlen(init) +80 > result_len) {
> + Â Â Â Â Â Â Â (void) attrset(attributes[FUNCTION_HIGHLIGHT] | A_BLINK);
> + Â Â Â Â Â Â Â mvprintw(0, 0, "result_len(%d) not enough to contain
> init_strlen(%d) in %s:%d:%s\n",
> + Â Â Â Â Â Â Â Â Â Â Â Â result_len, strlen(init),
> + Â Â Â Â Â Â Â Â Â Â Â Â __FILE__, __LINE__, __func__);
> + Â Â Â Â Â Â Â flash();
> + Â Â Â }
>
> Â Â Â Â /* find the widest line of msg: */
> Â Â Â Â prompt_lines = get_line_no(prompt);
> @@ -405,7 +413,11 @@ int dialog_inputbox(WINDOW *main_window,
> Â Â Â Â fill_window(prompt_win, prompt);
>
> Â Â Â Â mvwprintw(form_win, 0, 0, "%*s", prompt_width, " ");
> - Â Â Â mvwprintw(form_win, 0, 0, "%s", result);
> + Â Â Â cursor_form_win = min(cursor_position, prompt_width-1);
> + Â Â Â mvwprintw(form_win, 0, 0, "%s",
> + Â Â Â Â Â Â Â Â result + cursor_position-cursor_form_win);
> + Â Â Â mvprintw(0, 0, "cursor_position=%d, cursor_form_win=%d, prompt_width=%d\n",
> + Â Â Â Â Â Â Â Â cursor_position, cursor_form_win, prompt_width);
>
> Â Â Â Â /* create panels */
> Â Â Â Â panel = new_panel(win);
> @@ -416,6 +428,8 @@ int dialog_inputbox(WINDOW *main_window,
> Â Â Â Â touchwin(win);
> Â Â Â Â refresh_all_windows(main_window);
> Â Â Â Â while ((res = wgetch(form_win))) {
> + Â Â Â Â Â Â Â mvprintw(0, 0, "got key: %d\n", res);
> +
> Â Â Â Â Â Â Â Â int len = strlen(result);
> Â Â Â Â Â Â Â Â switch (res) {
> Â Â Â Â Â Â Â Â case 10: /* ENTER */
> @@ -431,6 +445,13 @@ int dialog_inputbox(WINDOW *main_window,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â &result[cursor_position],
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â len-cursor_position+1);
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â cursor_position--;
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (cursor_form_win < 5
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â && cursor_position > 5)
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â cursor_form_win += prompt_width/2;
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â else
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â cursor_form_win--;
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (cursor_form_win > cursor_position)
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â cursor_form_win = cursor_position;
> Â Â Â Â Â Â Â Â Â Â Â Â }
> Â Â Â Â Â Â Â Â Â Â Â Â break;
> Â Â Â Â Â Â Â Â case KEY_DC:
> @@ -440,16 +461,41 @@ int dialog_inputbox(WINDOW *main_window,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â len-cursor_position+1);
> Â Â Â Â Â Â Â Â Â Â Â Â }
> Â Â Â Â Â Â Â Â Â Â Â Â break;
> - Â Â Â Â Â Â Â case KEY_UP:
> + Â Â Â Â Â Â Â case 6: /* Ctrl-F */
> Â Â Â Â Â Â Â Â case KEY_RIGHT:
> - Â Â Â Â Â Â Â Â Â Â Â if (cursor_position < len &&
> - Â Â Â Â Â Â Â Â Â Â Â Â Â cursor_position < min(result_len, prompt_width))
> + Â Â Â Â Â Â Â Â Â Â Â if (cursor_position < len) {
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â cursor_position++;
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (cursor_form_win > prompt_width-5
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â && cursor_position < len-5)
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â cursor_form_win -= prompt_width/2;
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â else
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â cursor_form_win++;
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (cursor_form_win < min(cursor_position, prompt_width-1) -
> (len-cursor_position))
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â cursor_form_win = min(cursor_position, prompt_width-1) -
> (len-cursor_position);
> + Â Â Â Â Â Â Â Â Â Â Â }
> Â Â Â Â Â Â Â Â Â Â Â Â break;
> - Â Â Â Â Â Â Â case KEY_DOWN:
> + Â Â Â Â Â Â Â case 2: /* Ctrl-B */
> Â Â Â Â Â Â Â Â case KEY_LEFT:
> - Â Â Â Â Â Â Â Â Â Â Â if (cursor_position > 0)
> + Â Â Â Â Â Â Â Â Â Â Â if (cursor_position > 0) {
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â cursor_position--;
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (cursor_form_win < 5
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â && cursor_position > 5)
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â cursor_form_win += prompt_width/2;
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â else
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â cursor_form_win--;
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (cursor_form_win > cursor_position)
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â cursor_form_win = cursor_position;
> + Â Â Â Â Â Â Â Â Â Â Â }
> + Â Â Â Â Â Â Â Â Â Â Â break;
> + Â Â Â Â Â Â Â case 1: /* Ctrl-A */
> + Â Â Â Â Â Â Â case KEY_HOME:
> + Â Â Â Â Â Â Â Â Â Â Â cursor_position = 0;
> + Â Â Â Â Â Â Â Â Â Â Â cursor_form_win = 0;
> + Â Â Â Â Â Â Â Â Â Â Â break;
> + Â Â Â Â Â Â Â case 5: /* Ctrl-E */
> + Â Â Â Â Â Â Â case KEY_END:
> + Â Â Â Â Â Â Â Â Â Â Â cursor_position = len;
> + Â Â Â Â Â Â Â Â Â Â Â cursor_form_win = min(cursor_position, prompt_width-1);
> Â Â Â Â Â Â Â Â Â Â Â Â break;
> Â Â Â Â Â Â Â Â default:
> Â Â Â Â Â Â Â Â Â Â Â Â if ((isgraph(res) || isspace(res)) &&
> @@ -457,19 +503,24 @@ int dialog_inputbox(WINDOW *main_window,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â /* insert the char at the proper position */
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â memmove(&result[cursor_position+1],
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â &result[cursor_position],
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â len+1);
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â len-cursor_position+1);
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â result[cursor_position] = res;
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â cursor_position++;
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (cursor_form_win < prompt_width-1)
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â cursor_form_win++;
> Â Â Â Â Â Â Â Â Â Â Â Â } else {
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â mvprintw(0, 0, "unknow key: %d\n", res);
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â mvprintw(0, 0, "unknown key: %d\n", res);
> Â Â Â Â Â Â Â Â Â Â Â Â }
> Â Â Â Â Â Â Â Â Â Â Â Â break;
> Â Â Â Â Â Â Â Â }
> + Â Â Â Â Â Â Â mvprintw(0, 0, "got key: %d, cursor_position=%d, cursor_form_win=%d\n",
> + Â Â Â Â Â Â Â Â Â Â Â Â res, cursor_position, cursor_form_win);
> Â Â Â Â Â Â Â Â wmove(form_win, 0, 0);
> Â Â Â Â Â Â Â Â wclrtoeol(form_win);
> Â Â Â Â Â Â Â Â mvwprintw(form_win, 0, 0, "%*s", prompt_width, " ");
> - Â Â Â Â Â Â Â mvwprintw(form_win, 0, 0, "%s", result);
> - Â Â Â Â Â Â Â wmove(form_win, 0, cursor_position);
> + Â Â Â Â Â Â Â mvwprintw(form_win, 0, 0, "%s",
> + Â Â Â Â Â Â Â Â Â Â Â Â result + cursor_position-cursor_form_win);
> + Â Â Â Â Â Â Â wmove(form_win, 0, cursor_form_win);
> Â Â Â Â Â Â Â Â touchwin(win);
> Â Â Â Â Â Â Â Â refresh_all_windows(main_window);
>
>
> --
> Cheng Renquan (ç¨ä»»å
¨)
> --
> 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
>
ÿôèº{.nÇ+·®+%Ëÿ±éݶ\x17¥wÿº{.nÇ+·¥{±þFîWÿ{ayº\x1dÊÚë,j\a¢f£¢·hïêÿêçz_è®\x03(éÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?¨èÚ&£ø§~á
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [RFC] nconf bug fixes and improvements
2011-08-29 14:14 ` Arnaud Lacombe
(?)
@ 2011-08-29 16:12 ` Randy Dunlap
-1 siblings, 0 replies; 35+ messages in thread
From: Randy Dunlap @ 2011-08-29 16:12 UTC (permalink / raw)
To: Arnaud Lacombe
Cc: Cheng Renquan, Michal Marek, linux-kbuild, linux-kernel, Nir Tzachar
On Mon, 29 Aug 2011 10:14:00 -0400 Arnaud Lacombe wrote:
> Hi,
>
> On Mon, Aug 29, 2011 at 5:09 AM, Cheng Renquan <crquan@gmail.com> wrote:
> > bug fixes:
> >
> Please split diff in logical changes, one patch per issue. This is
> unreviewable in the current state. And please, use git.
Agree with all except "use git". This is a small patch (series).
git is not needed.
> See `Documentation/SubmittingPatches' for further informations.
At least the patch path names need to be fixed.
> Thanks,
> - Arnaud
>
> > 1) char dialog_input_result[256]; is not enough for config item like:
> > CONFIG_EXTRA_FIRMWARE="iwlwifi-100-5.ucode iwlwifi-1000-3.ucode
> > iwlwifi-3945-2.ucode iwlwifi-4965-2.ucode iwlwifi-5000-1.ucode
> > iwlwifi-5000-2.ucode iwlwifi-5150-2.ucode iwlwifi-6000-4.ucode
> > iwlwifi-6000g2a-5.ucode iwlwifi-6000g2b-5.ucode iwlwifi-6050-4.ucode
> > iwlwifi-6050-5.ucode radeon/CEDAR_me.bin radeon/CEDAR_pfp.bin
> > radeon/CEDAR_rlc.bin radeon/CYPRESS_me.bin radeon/CYPRESS_pfp.bin
> > radeon/CYPRESS_rlc.bin radeon/JUNIPER_me.bin radeon/JUNIPER_pfp.bin
> > radeon/JUNIPER_rlc.bin radeon/R600_rlc.bin radeon/R700_rlc.bin
> > radeon/REDWOOD_me.bin radeon/REDWOOD_pfp.bin radeon/REDWOOD_rlc.bin"
> >
> > the original nconf just stack overflow / crashed when dealing with
> > longer than 256 bytes strings; Since the original menuconfig also just
> > uses a fixed length buffer [MAX_LEN=2048] which works for the years,
> > here I just append a 0 make it work in the easiest way; if required,
> > it could also be changed to a dynamically allocated buffer;
> >
> > char dialog_input_result[MAX_LEN + 1];
> >
> > 2) memmove's 3rd argument should be len-cursor_position+1, the
> > original len+1 may cause segment fault in theory;
> > memmove(&result[cursor_position+1],
> > &result[cursor_position],
> > - len+1);
> > + len-cursor_position+1);
> >
> > 3) typo:
> >
> > - mvprintw(0, 0, "unknow key: %d\n", res);
> > + mvprintw(0, 0, "unknown key: %d\n", res);
> >
> >
> > improvement:
> > 1) its original conf_string doesn't work with longer string values
> > (longer than its dialog box width), not at all
> > (or may work in an invisible way if anyone has tried that)
> > Here I added a new variable cursor_form_win to record that new state:
> > cursor of the input box; and make it fun:
> > when you move cursor to almost left/right edge, it auto adjust text
> > to center, by half prompt box width;
> >
> > 2) Remove KEY_UP as RIGHT and KEY_DOWN as LEFT,
> > Add Home/End to locate the string begin/end;
> > Emacs-like key bind (C-a/C-e as Home/End, C-f/C-b as forward/backward);
> > this keybind I'd like but may be controversial, it could be
> > discussed and separated;
> >
> > This is just [Request for Comments], it just works here on one of my
> > distributor kernels,
> > if anyone may think it's useful, please feedback and I would like to
> > split it into
> > patch series and rebase to linus latest branch;
---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [RFC] nconf bug fixes and improvements
2011-08-29 9:09 [RFC] nconf bug fixes and improvements Cheng Renquan
2011-08-29 14:14 ` Arnaud Lacombe
@ 2011-08-29 16:53 ` Sam Ravnborg
2011-08-29 23:56 ` [PATCH 1/5] scripts/kconfig/nconf: fix typo: unknow => unknown Cheng Renquan
` (2 subsequent siblings)
4 siblings, 0 replies; 35+ messages in thread
From: Sam Ravnborg @ 2011-08-29 16:53 UTC (permalink / raw)
To: Cheng Renquan; +Cc: Michal Marek, linux-kbuild, linux-kernel, Nir Tzachar
Hi Cheng.
On Mon, Aug 29, 2011 at 02:09:59AM -0700, Cheng Renquan wrote:
> bug fixes:
> 1) char dialog_input_result[256]; is not enough for config item like:
> CONFIG_EXTRA_FIRMWARE="iwlwifi-100-5.ucode iwlwifi-1000-3.ucode
> iwlwifi-3945-2.ucode iwlwifi-4965-2.ucode iwlwifi-5000-1.ucode
> iwlwifi-5000-2.ucode iwlwifi-5150-2.ucode iwlwifi-6000-4.ucode
> iwlwifi-6000g2a-5.ucode iwlwifi-6000g2b-5.ucode iwlwifi-6050-4.ucode
> iwlwifi-6050-5.ucode radeon/CEDAR_me.bin radeon/CEDAR_pfp.bin
> radeon/CEDAR_rlc.bin radeon/CYPRESS_me.bin radeon/CYPRESS_pfp.bin
> radeon/CYPRESS_rlc.bin radeon/JUNIPER_me.bin radeon/JUNIPER_pfp.bin
> radeon/JUNIPER_rlc.bin radeon/R600_rlc.bin radeon/R700_rlc.bin
> radeon/REDWOOD_me.bin radeon/REDWOOD_pfp.bin radeon/REDWOOD_rlc.bin"
>
> the original nconf just stack overflow / crashed when dealing with
> longer than 256 bytes strings; Since the original menuconfig also just
> uses a fixed length buffer [MAX_LEN=2048] which works for the years,
> here I just append a 0 make it work in the easiest way; if required,
> it could also be changed to a dynamically allocated buffer;
>
> char dialog_input_result[MAX_LEN + 1];
Please do not repeat errors from the past.
Fix this to allocate the string so we handle strings of
unlimited length.
> 2) memmove's 3rd argument should be len-cursor_position+1, the
> original len+1 may cause segment fault in theory;
> memmove(&result[cursor_position+1],
> &result[cursor_position],
> - len+1);
> + len-cursor_position+1);
Looks OK - but I did not check in detail.
>
> 3) typo:
>
> - mvprintw(0, 0, "unknow key: %d\n", res);
> + mvprintw(0, 0, "unknown key: %d\n", res);
>
ACK
>
> improvement:
> 1) its original conf_string doesn't work with longer string values
> (longer than its dialog box width), not at all
> (or may work in an invisible way if anyone has tried that)
> Here I added a new variable cursor_form_win to record that new state:
> cursor of the input box; and make it fun:
> when you move cursor to almost left/right edge, it auto adjust text
> to center, by half prompt box width;
Sounds resonable - but did not try it.
>
> 2) Remove KEY_UP as RIGHT and KEY_DOWN as LEFT,
Why?
> Add Home/End to locate the string begin/end;
OK
> Emacs-like key bind (C-a/C-e as Home/End, C-f/C-b as forward/backward);
No other part of nconf is emacs-lke - so please no.
> --- /mnt/static/usr/src/linux-3.0-gentoo-r1/scripts/kconfig/nconf.gui.c 2011-07-21
> 19:17:23.000000000 -0700
> +++ /mnt/dynamic/usr/src/linux-3.0-gentoo-r1/scripts/kconfig/nconf.gui.c 2011-08-28
> 18:00:56.441862565 -0700
> @@ -367,7 +367,15 @@ int dialog_inputbox(WINDOW *main_window,
> int i, x, y;
> int res = -1;
> int cursor_position = strlen(init);
> + int cursor_form_win;
>
> + if (strlen(init) +80 > result_len) {
No hardcoded number please. And be consistent with use of spaces.
> + (void) attrset(attributes[FUNCTION_HIGHLIGHT] | A_BLINK);
> + mvprintw(0, 0, "result_len(%d) not enough to contain
> init_strlen(%d) in %s:%d:%s\n",
> + result_len, strlen(init),
> + __FILE__, __LINE__, __func__);
> + flash();
> + }
What is the purpose of this statement - does not look clear to me.
Did not look at the rest of the patch - awiting a split-up version.
Sam
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 1/5] scripts/kconfig/nconf: fix typo: unknow => unknown
2011-08-29 9:09 [RFC] nconf bug fixes and improvements Cheng Renquan
2011-08-29 14:14 ` Arnaud Lacombe
2011-08-29 16:53 ` Sam Ravnborg
@ 2011-08-29 23:56 ` Cheng Renquan
2011-08-29 23:56 ` [PATCH 2/5] scripts/kconfig/nconf: fix memmove's length arg Cheng Renquan
2011-08-30 0:02 ` [PATCH 1/5] scripts/kconfig/nconf: fix typo: unknow => unknown Arnaud Lacombe
2011-08-31 7:46 ` [PATCH 0/6] nconf bug fixes and U/E improvements Cheng Renquan
2011-09-01 17:52 ` [PATCH V3 1/5] scripts/kconfig/nconf: fix typo: unknow => unknown crquan
4 siblings, 2 replies; 35+ messages in thread
From: Cheng Renquan @ 2011-08-29 23:56 UTC (permalink / raw)
To: linux-kbuild
Cc: Sam Ravnborg, Michal Marek, Nir Tzachar, Randy Dunlap, c.rq541
Signed-off-by: Cheng Renquan <crquan@gmail.com>
---
scripts/kconfig/nconf.gui.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/scripts/kconfig/nconf.gui.c b/scripts/kconfig/nconf.gui.c
index f8137b3..d3af04e 100644
--- a/scripts/kconfig/nconf.gui.c
+++ b/scripts/kconfig/nconf.gui.c
@@ -461,7 +461,7 @@ int dialog_inputbox(WINDOW *main_window,
result[cursor_position] = res;
cursor_position++;
} else {
- mvprintw(0, 0, "unknow key: %d\n", res);
+ mvprintw(0, 0, "unknown key: %d\n", res);
}
break;
}
--
1.7.6
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH 2/5] scripts/kconfig/nconf: fix memmove's length arg
2011-08-29 23:56 ` [PATCH 1/5] scripts/kconfig/nconf: fix typo: unknow => unknown Cheng Renquan
@ 2011-08-29 23:56 ` Cheng Renquan
2011-08-29 23:56 ` [PATCH 3/5] scripts/kconfig/nconf: dynamically alloc dialog_input_result Cheng Renquan
2011-08-30 0:02 ` [PATCH 1/5] scripts/kconfig/nconf: fix typo: unknow => unknown Arnaud Lacombe
1 sibling, 1 reply; 35+ messages in thread
From: Cheng Renquan @ 2011-08-29 23:56 UTC (permalink / raw)
To: linux-kbuild
Cc: Sam Ravnborg, Michal Marek, Nir Tzachar, Randy Dunlap, c.rq541
In case KEY_BACKSPACE or KEY_DC to delete a char, it memmove only
(len-cursor_position+1) bytes;
the default case is to insert a char, it should also memmove only
(len-cursor_position+1) bytes;
the original use of (len+1) is wrong and may access following memory
that doesn't belong to result, may cause SegFault in theroy;
case KEY_BACKSPACE:
if (cursor_position > 0) {
memmove(&result[cursor_position-1],
&result[cursor_position],
len-cursor_position+1);
cursor_position--;
}
break;
case KEY_DC:
if (cursor_position >= 0 && cursor_position < len) {
memmove(&result[cursor_position],
&result[cursor_position+1],
len-cursor_position+1);
}
break;
default:
if ((isgraph(res) || isspace(res)) &&
len-2 < result_len) {
/* insert the char at the proper position */
memmove(&result[cursor_position+1],
&result[cursor_position],
len-cursor_position+1);
result[cursor_position] = res;
cursor_position++;
}
Signed-off-by: Cheng Renquan <crquan@gmail.com>
---
scripts/kconfig/nconf.gui.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/scripts/kconfig/nconf.gui.c b/scripts/kconfig/nconf.gui.c
index d3af04e..3ce2a7c 100644
--- a/scripts/kconfig/nconf.gui.c
+++ b/scripts/kconfig/nconf.gui.c
@@ -457,7 +457,7 @@ int dialog_inputbox(WINDOW *main_window,
/* insert the char at the proper position */
memmove(&result[cursor_position+1],
&result[cursor_position],
- len+1);
+ len-cursor_position+1);
result[cursor_position] = res;
cursor_position++;
} else {
--
1.7.6
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH 3/5] scripts/kconfig/nconf: dynamically alloc dialog_input_result
2011-08-29 23:56 ` [PATCH 2/5] scripts/kconfig/nconf: fix memmove's length arg Cheng Renquan
@ 2011-08-29 23:56 ` Cheng Renquan
2011-08-29 23:56 ` [PATCH 4/5] scripts/kconfig/nconf: fix editing long strings Cheng Renquan
` (3 more replies)
0 siblings, 4 replies; 35+ messages in thread
From: Cheng Renquan @ 2011-08-29 23:56 UTC (permalink / raw)
To: linux-kbuild
Cc: Sam Ravnborg, Michal Marek, Nir Tzachar, Randy Dunlap, c.rq541
to support unlimited length string config items;
Signed-off-by: Cheng Renquan <crquan@gmail.com>
---
scripts/kconfig/nconf.c | 24 ++++++++++++++++--------
scripts/kconfig/nconf.gui.c | 25 ++++++++++++++++++++-----
scripts/kconfig/nconf.h | 2 +-
3 files changed, 37 insertions(+), 14 deletions(-)
diff --git a/scripts/kconfig/nconf.c b/scripts/kconfig/nconf.c
index 39ca1f1..ee37358 100644
--- a/scripts/kconfig/nconf.c
+++ b/scripts/kconfig/nconf.c
@@ -280,6 +280,9 @@ static int global_exit;
/* the currently selected button */
const char *current_instructions = menu_instructions;
+static char *dialog_input_result;
+static int dialog_input_result_len;
+
static void conf(struct menu *menu);
static void conf_choice(struct menu *menu);
static void conf_string(struct menu *menu);
@@ -695,7 +698,6 @@ static void search_conf(void)
{
struct symbol **sym_arr;
struct gstr res;
- char dialog_input_result[100];
char *dialog_input;
int dres;
again:
@@ -703,7 +705,7 @@ again:
_("Search Configuration Parameter"),
_("Enter " CONFIG_ " (sub)string to search for "
"(with or without \"" CONFIG_ "\")"),
- "", dialog_input_result, 99);
+ "", dialog_input_result, &dialog_input_result_len);
switch (dres) {
case 0:
break;
@@ -1348,7 +1350,6 @@ static void conf_choice(struct menu *menu)
static void conf_string(struct menu *menu)
{
const char *prompt = menu_get_prompt(menu);
- char dialog_input_result[256];
while (1) {
int res;
@@ -1372,7 +1373,7 @@ static void conf_string(struct menu *menu)
heading,
sym_get_string_value(menu->sym),
dialog_input_result,
- sizeof(dialog_input_result));
+ &dialog_input_result_len);
switch (res) {
case 0:
if (sym_set_string_value(menu->sym,
@@ -1392,14 +1393,13 @@ static void conf_string(struct menu *menu)
static void conf_load(void)
{
- char dialog_input_result[256];
while (1) {
int res;
res = dialog_inputbox(main_window,
NULL, load_config_text,
filename,
dialog_input_result,
- sizeof(dialog_input_result));
+ &dialog_input_result_len);
switch (res) {
case 0:
if (!dialog_input_result[0])
@@ -1424,14 +1424,13 @@ static void conf_load(void)
static void conf_save(void)
{
- char dialog_input_result[256];
while (1) {
int res;
res = dialog_inputbox(main_window,
NULL, save_config_text,
filename,
dialog_input_result,
- sizeof(dialog_input_result));
+ &dialog_input_result_len);
switch (res) {
case 0:
if (!dialog_input_result[0])
@@ -1488,6 +1487,15 @@ int main(int ac, char **av)
single_menu_mode = 1;
}
+ /* initially alloc 2048 bytes for dialog_input_result */
+ dialog_input_result_len = 2048;
+ dialog_input_result = malloc(dialog_input_result_len);
+ if (!dialog_input_result) {
+ fprintf(stderr, "Not enough memory for dialog_input_result(%d)\n",
+ dialog_input_result_len);
+ exit(1);
+ }
+
/* Initialize curses */
initscr();
/* set color theme */
diff --git a/scripts/kconfig/nconf.gui.c b/scripts/kconfig/nconf.gui.c
index 3ce2a7c..bc482ad 100644
--- a/scripts/kconfig/nconf.gui.c
+++ b/scripts/kconfig/nconf.gui.c
@@ -356,7 +356,7 @@ int btn_dialog(WINDOW *main_window, const char *msg, int btn_num, ...)
int dialog_inputbox(WINDOW *main_window,
const char *title, const char *prompt,
- const char *init, char *result, int result_len)
+ const char *init, char *result, int *result_len)
{
int prompt_lines = 0;
int prompt_width = 0;
@@ -368,6 +368,14 @@ int dialog_inputbox(WINDOW *main_window,
int res = -1;
int cursor_position = strlen(init);
+ if (strlen(init) > *result_len) {
+ do {
+ *result_len *= 2;
+ } while (strlen(init) > *result_len);
+ result = realloc(result, *result_len);
+ /* here didn't check result, if it's NULL,
+ just let it silently fail (SegFault) */
+ }
/* find the widest line of msg: */
prompt_lines = get_line_no(prompt);
@@ -384,7 +392,7 @@ int dialog_inputbox(WINDOW *main_window,
y = (LINES-(prompt_lines+4))/2;
x = (COLS-(prompt_width+4))/2;
- strncpy(result, init, result_len);
+ strncpy(result, init, *result_len);
/* create the windows */
win = newwin(prompt_lines+6, prompt_width+7, y, x);
@@ -443,7 +451,7 @@ int dialog_inputbox(WINDOW *main_window,
case KEY_UP:
case KEY_RIGHT:
if (cursor_position < len &&
- cursor_position < min(result_len, prompt_width))
+ cursor_position < min(*result_len, prompt_width))
cursor_position++;
break;
case KEY_DOWN:
@@ -452,8 +460,15 @@ int dialog_inputbox(WINDOW *main_window,
cursor_position--;
break;
default:
- if ((isgraph(res) || isspace(res)) &&
- len-2 < result_len) {
+ if ((isgraph(res) || isspace(res))) {
+ /* one for new char, one for '\0' */
+ if (len+2 > *result_len) {
+ do {
+ *result_len *= 2;
+ } while (len+2 > *result_len);
+ result = realloc(result, *result_len);
+ /* silently fail in the same way above */
+ }
/* insert the char at the proper position */
memmove(&result[cursor_position+1],
&result[cursor_position],
diff --git a/scripts/kconfig/nconf.h b/scripts/kconfig/nconf.h
index 58fbda8..98b2c23 100644
--- a/scripts/kconfig/nconf.h
+++ b/scripts/kconfig/nconf.h
@@ -89,7 +89,7 @@ void fill_window(WINDOW *win, const char *text);
int btn_dialog(WINDOW *main_window, const char *msg, int btn_num, ...);
int dialog_inputbox(WINDOW *main_window,
const char *title, const char *prompt,
- const char *init, char *result, int result_len);
+ const char *init, char *result, int *result_len);
void refresh_all_windows(WINDOW *main_window);
void show_scroll_win(WINDOW *main_window,
const char *title,
--
1.7.6
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH 4/5] scripts/kconfig/nconf: fix editing long strings
2011-08-29 23:56 ` [PATCH 3/5] scripts/kconfig/nconf: dynamically alloc dialog_input_result Cheng Renquan
@ 2011-08-29 23:56 ` Cheng Renquan
2011-08-29 23:56 ` [PATCH 5/5] scripts/kconfig/nconf: add KEY_HOME / KEY_END for dialog_inputbox Cheng Renquan
2011-08-30 4:59 ` [PATCH 4/5] scripts/kconfig/nconf: fix editing long strings Arnaud Lacombe
2011-08-30 0:16 ` [PATCH 3/5] scripts/kconfig/nconf: dynamically alloc dialog_input_result Cheng Renquan
` (2 subsequent siblings)
3 siblings, 2 replies; 35+ messages in thread
From: Cheng Renquan @ 2011-08-29 23:56 UTC (permalink / raw)
To: linux-kbuild
Cc: Sam Ravnborg, Michal Marek, Nir Tzachar, Randy Dunlap, c.rq541
The original dialog_inputbox doesn't work with longer than prompt_width
strings, here fixed it in this way:
1) add variable cursor_form_win to record cursor of form_win,
keep its value always between [0, prompt_width-1];
keep the original cursor_position as cursor of the string result,
for short strings, cursor_form_win is identical to cursor_position;
for long strings, use (cursor_position-cursor_form_win) as begin offset
to show part of the string in form_win;
2) whenever cursor of form_win is near (by 3 chars) to left or right edge
of form_win, make a auto scroll by half prompt_width, to make this one
line string editor more fun to use;
3) update len for later cursor_form_win correct calculation;
Signed-off-by: Cheng Renquan <crquan@gmail.com>
---
scripts/kconfig/nconf.gui.c | 43 +++++++++++++++++++++++++++++++++++++------
1 files changed, 37 insertions(+), 6 deletions(-)
diff --git a/scripts/kconfig/nconf.gui.c b/scripts/kconfig/nconf.gui.c
index bc482ad..62a41d1 100644
--- a/scripts/kconfig/nconf.gui.c
+++ b/scripts/kconfig/nconf.gui.c
@@ -367,6 +367,7 @@ int dialog_inputbox(WINDOW *main_window,
int i, x, y;
int res = -1;
int cursor_position = strlen(init);
+ int cursor_form_win;
if (strlen(init) > *result_len) {
do {
@@ -413,7 +414,9 @@ int dialog_inputbox(WINDOW *main_window,
fill_window(prompt_win, prompt);
mvwprintw(form_win, 0, 0, "%*s", prompt_width, " ");
- mvwprintw(form_win, 0, 0, "%s", result);
+ cursor_form_win = min(cursor_position, prompt_width-1);
+ mvwprintw(form_win, 0, 0, "%s",
+ result + cursor_position-cursor_form_win);
/* create panels */
panel = new_panel(win);
@@ -439,6 +442,8 @@ int dialog_inputbox(WINDOW *main_window,
&result[cursor_position],
len-cursor_position+1);
cursor_position--;
+ cursor_form_win--;
+ len--;
}
break;
case KEY_DC:
@@ -446,18 +451,22 @@ int dialog_inputbox(WINDOW *main_window,
memmove(&result[cursor_position],
&result[cursor_position+1],
len-cursor_position+1);
+ len--;
}
break;
case KEY_UP:
case KEY_RIGHT:
- if (cursor_position < len &&
- cursor_position < min(*result_len, prompt_width))
+ if (cursor_position < len) {
cursor_position++;
+ cursor_form_win++;
+ }
break;
case KEY_DOWN:
case KEY_LEFT:
- if (cursor_position > 0)
+ if (cursor_position > 0) {
cursor_position--;
+ cursor_form_win--;
+ }
break;
default:
if ((isgraph(res) || isspace(res))) {
@@ -475,16 +484,38 @@ int dialog_inputbox(WINDOW *main_window,
len-cursor_position+1);
result[cursor_position] = res;
cursor_position++;
+ cursor_form_win++;
+ len++;
} else {
mvprintw(0, 0, "unknown key: %d\n", res);
}
break;
}
+ if (len <= prompt_width-1)
+ cursor_form_win = cursor_position;
+ else {
+ if (cursor_form_win <= 3)
+ cursor_form_win += prompt_width/2;
+ else if (cursor_form_win >= prompt_width-3)
+ cursor_form_win -= prompt_width/2;
+
+ if (cursor_form_win < 0)
+ cursor_form_win = 0;
+ else if (cursor_form_win >= prompt_width-1)
+ cursor_form_win = prompt_width-1;
+
+ if (cursor_form_win > cursor_position)
+ cursor_form_win = cursor_position;
+ if (cursor_form_win < (prompt_width-1) - (len-cursor_position))
+ cursor_form_win = (prompt_width-1) - (len-cursor_position);
+ }
+
wmove(form_win, 0, 0);
wclrtoeol(form_win);
mvwprintw(form_win, 0, 0, "%*s", prompt_width, " ");
- mvwprintw(form_win, 0, 0, "%s", result);
- wmove(form_win, 0, cursor_position);
+ mvwprintw(form_win, 0, 0, "%s",
+ result + cursor_position-cursor_form_win);
+ wmove(form_win, 0, cursor_form_win);
touchwin(win);
refresh_all_windows(main_window);
--
1.7.6
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH 5/5] scripts/kconfig/nconf: add KEY_HOME / KEY_END for dialog_inputbox
2011-08-29 23:56 ` [PATCH 4/5] scripts/kconfig/nconf: fix editing long strings Cheng Renquan
@ 2011-08-29 23:56 ` Cheng Renquan
2011-08-30 4:59 ` [PATCH 4/5] scripts/kconfig/nconf: fix editing long strings Arnaud Lacombe
1 sibling, 0 replies; 35+ messages in thread
From: Cheng Renquan @ 2011-08-29 23:56 UTC (permalink / raw)
To: linux-kbuild
Cc: Sam Ravnborg, Michal Marek, Nir Tzachar, Randy Dunlap, c.rq541
to make it better when editing long strings;
Signed-off-by: Cheng Renquan <crquan@gmail.com>
---
scripts/kconfig/nconf.gui.c | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/scripts/kconfig/nconf.gui.c b/scripts/kconfig/nconf.gui.c
index 62a41d1..909c85f 100644
--- a/scripts/kconfig/nconf.gui.c
+++ b/scripts/kconfig/nconf.gui.c
@@ -468,6 +468,14 @@ int dialog_inputbox(WINDOW *main_window,
cursor_form_win--;
}
break;
+ case KEY_HOME:
+ cursor_position = 0;
+ cursor_form_win = 0;
+ break;
+ case KEY_END:
+ cursor_position = len;
+ cursor_form_win = min(cursor_position, prompt_width-1);
+ break;
default:
if ((isgraph(res) || isspace(res))) {
/* one for new char, one for '\0' */
--
1.7.6
^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [PATCH 1/5] scripts/kconfig/nconf: fix typo: unknow => unknown
2011-08-29 23:56 ` [PATCH 1/5] scripts/kconfig/nconf: fix typo: unknow => unknown Cheng Renquan
2011-08-29 23:56 ` [PATCH 2/5] scripts/kconfig/nconf: fix memmove's length arg Cheng Renquan
@ 2011-08-30 0:02 ` Arnaud Lacombe
1 sibling, 0 replies; 35+ messages in thread
From: Arnaud Lacombe @ 2011-08-30 0:02 UTC (permalink / raw)
To: Cheng Renquan
Cc: linux-kbuild, Sam Ravnborg, Michal Marek, Nir Tzachar,
Randy Dunlap, c.rq541
Hi,
On Mon, Aug 29, 2011 at 7:56 PM, Cheng Renquan <crquan@gmail.com> wrote:
> Signed-off-by: Cheng Renquan <crquan@gmail.com>
ACK.
Thanks,
- Arnaud
> ---
> scripts/kconfig/nconf.gui.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/scripts/kconfig/nconf.gui.c b/scripts/kconfig/nconf.gui.c
> index f8137b3..d3af04e 100644
> --- a/scripts/kconfig/nconf.gui.c
> +++ b/scripts/kconfig/nconf.gui.c
> @@ -461,7 +461,7 @@ int dialog_inputbox(WINDOW *main_window,
> result[cursor_position] = res;
> cursor_position++;
> } else {
> - mvprintw(0, 0, "unknow key: %d\n", res);
> + mvprintw(0, 0, "unknown key: %d\n", res);
> }
> break;
> }
> --
> 1.7.6
>
> --
> 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
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 3/5] scripts/kconfig/nconf: dynamically alloc dialog_input_result
2011-08-29 23:56 ` [PATCH 3/5] scripts/kconfig/nconf: dynamically alloc dialog_input_result Cheng Renquan
2011-08-29 23:56 ` [PATCH 4/5] scripts/kconfig/nconf: fix editing long strings Cheng Renquan
@ 2011-08-30 0:16 ` Cheng Renquan
2011-08-30 3:25 ` Arnaud Lacombe
2011-08-30 4:14 ` Arnaud Lacombe
3 siblings, 0 replies; 35+ messages in thread
From: Cheng Renquan @ 2011-08-30 0:16 UTC (permalink / raw)
To: linux-kbuild
Cc: Sam Ravnborg, Michal Marek, Nir Tzachar, Randy Dunlap, c.rq541
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=UTF-8, Size: 3978 bytes --]
On Mon, Aug 29, 2011 at 4:56 PM, Cheng Renquan <crquan@gmail.com> wrote:
> to support unlimited length string config items;
>
> Signed-off-by: Cheng Renquan <crquan@gmail.com>
> ---
>  scripts/kconfig/nconf.c   |  24 ++++++++++++++++--------
> Â scripts/kconfig/nconf.gui.c | Â 25 ++++++++++++++++++++-----
>  scripts/kconfig/nconf.h   |   2 +-
> Â 3 files changed, 37 insertions(+), 14 deletions(-)
> diff --git a/scripts/kconfig/nconf.gui.c b/scripts/kconfig/nconf.gui.c
> index 3ce2a7c..bc482ad 100644
> --- a/scripts/kconfig/nconf.gui.c
> +++ b/scripts/kconfig/nconf.gui.c
> @@ -356,7 +356,7 @@ int btn_dialog(WINDOW *main_window, const char *msg, int btn_num, ...)
>
> Â int dialog_inputbox(WINDOW *main_window,
> Â Â Â Â Â Â Â Â const char *title, const char *prompt,
> - Â Â Â Â Â Â Â const char *init, char *result, int result_len)
> + Â Â Â Â Â Â Â const char *init, char *result, int *result_len)
> Â {
> Â Â Â Â int prompt_lines = 0;
> Â Â Â Â int prompt_width = 0;
> @@ -368,6 +368,14 @@ int dialog_inputbox(WINDOW *main_window,
> Â Â Â Â int res = -1;
> Â Â Â Â int cursor_position = strlen(init);
>
> + Â Â Â if (strlen(init) > *result_len) {
> + Â Â Â Â Â Â Â do {
> + Â Â Â Â Â Â Â Â Â Â Â *result_len *= 2;
> + Â Â Â Â Â Â Â } while (strlen(init) > *result_len);
> + Â Â Â Â Â Â Â result = realloc(result, *result_len);
> + Â Â Â Â Â Â Â /* here didn't check result, if it's NULL,
> + Â Â Â Â Â Â Â Â Â just let it silently fail (SegFault) */
> + Â Â Â }
>
> Â Â Â Â /* find the widest line of msg: */
> Â Â Â Â prompt_lines = get_line_no(prompt);
> @@ -384,7 +392,7 @@ int dialog_inputbox(WINDOW *main_window,
> Â Â Â Â y = (LINES-(prompt_lines+4))/2;
> Â Â Â Â x = (COLS-(prompt_width+4))/2;
>
> - Â Â Â strncpy(result, init, result_len);
> + Â Â Â strncpy(result, init, *result_len);
>
> Â Â Â Â /* create the windows */
> Â Â Â Â win = newwin(prompt_lines+6, prompt_width+7, y, x);
> @@ -443,7 +451,7 @@ int dialog_inputbox(WINDOW *main_window,
> Â Â Â Â Â Â Â Â case KEY_UP:
> Â Â Â Â Â Â Â Â case KEY_RIGHT:
> Â Â Â Â Â Â Â Â Â Â Â Â if (cursor_position < len &&
> - Â Â Â Â Â Â Â Â Â Â Â Â Â cursor_position < min(result_len, prompt_width))
> + Â Â Â Â Â Â Â Â Â Â Â Â Â cursor_position < min(*result_len, prompt_width))
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â cursor_position++;
> Â Â Â Â Â Â Â Â Â Â Â Â break;
> Â Â Â Â Â Â Â Â case KEY_DOWN:
> @@ -452,8 +460,15 @@ int dialog_inputbox(WINDOW *main_window,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â cursor_position--;
> Â Â Â Â Â Â Â Â Â Â Â Â break;
> Â Â Â Â Â Â Â Â default:
> - Â Â Â Â Â Â Â Â Â Â Â if ((isgraph(res) || isspace(res)) &&
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â len-2 < result_len) {
> + Â Â Â Â Â Â Â Â Â Â Â if ((isgraph(res) || isspace(res))) {
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â /* one for new char, one for '\0' */
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (len+2 > *result_len) {
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â do {
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â *result_len *= 2;
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â } while (len+2 > *result_len);
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â result = realloc(result, *result_len);
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â /* silently fail in the same way above */
Sorry, here I really want to say "silently fail in the same way if
realloc returns NULL", or we could alternatively call exit(1) for a
graceful exit; also need to call ncurses cleanup first; but how about
silent fail?
ÿôèº{.nÇ+·®+%Ëÿ±éݶ\x17¥wÿº{.nÇ+·¥{±þFîWÿ{ayº\x1dÊÚë,j\a¢f£¢·hïêÿêçz_è®\x03(éÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?¨èÚ&£ø§~á
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 3/5] scripts/kconfig/nconf: dynamically alloc dialog_input_result
2011-08-29 23:56 ` [PATCH 3/5] scripts/kconfig/nconf: dynamically alloc dialog_input_result Cheng Renquan
2011-08-29 23:56 ` [PATCH 4/5] scripts/kconfig/nconf: fix editing long strings Cheng Renquan
2011-08-30 0:16 ` [PATCH 3/5] scripts/kconfig/nconf: dynamically alloc dialog_input_result Cheng Renquan
@ 2011-08-30 3:25 ` Arnaud Lacombe
2011-08-30 4:14 ` Arnaud Lacombe
3 siblings, 0 replies; 35+ messages in thread
From: Arnaud Lacombe @ 2011-08-30 3:25 UTC (permalink / raw)
To: Cheng Renquan
Cc: linux-kbuild, Sam Ravnborg, Michal Marek, Nir Tzachar,
Randy Dunlap, c.rq541
hi,
On Mon, Aug 29, 2011 at 7:56 PM, Cheng Renquan <crquan@gmail.com> wrote:
> to support unlimited length string config items;
>
> Signed-off-by: Cheng Renquan <crquan@gmail.com>
> ---
> scripts/kconfig/nconf.c | 24 ++++++++++++++++--------
> scripts/kconfig/nconf.gui.c | 25 ++++++++++++++++++++-----
> scripts/kconfig/nconf.h | 2 +-
> 3 files changed, 37 insertions(+), 14 deletions(-)
>
> diff --git a/scripts/kconfig/nconf.c b/scripts/kconfig/nconf.c
> index 39ca1f1..ee37358 100644
> --- a/scripts/kconfig/nconf.c
> +++ b/scripts/kconfig/nconf.c
> @@ -280,6 +280,9 @@ static int global_exit;
> /* the currently selected button */
> const char *current_instructions = menu_instructions;
>
> +static char *dialog_input_result;
> +static int dialog_input_result_len;
> +
> static void conf(struct menu *menu);
> static void conf_choice(struct menu *menu);
> static void conf_string(struct menu *menu);
> @@ -695,7 +698,6 @@ static void search_conf(void)
> {
> struct symbol **sym_arr;
> struct gstr res;
> - char dialog_input_result[100];
> char *dialog_input;
> int dres;
> again:
> @@ -703,7 +705,7 @@ again:
> _("Search Configuration Parameter"),
> _("Enter " CONFIG_ " (sub)string to search for "
> "(with or without \"" CONFIG_ "\")"),
> - "", dialog_input_result, 99);
> + "", dialog_input_result, &dialog_input_result_len);
> switch (dres) {
> case 0:
> break;
> @@ -1348,7 +1350,6 @@ static void conf_choice(struct menu *menu)
> static void conf_string(struct menu *menu)
> {
> const char *prompt = menu_get_prompt(menu);
> - char dialog_input_result[256];
>
> while (1) {
> int res;
> @@ -1372,7 +1373,7 @@ static void conf_string(struct menu *menu)
> heading,
> sym_get_string_value(menu->sym),
> dialog_input_result,
> - sizeof(dialog_input_result));
> + &dialog_input_result_len);
> switch (res) {
> case 0:
> if (sym_set_string_value(menu->sym,
> @@ -1392,14 +1393,13 @@ static void conf_string(struct menu *menu)
>
> static void conf_load(void)
> {
> - char dialog_input_result[256];
> while (1) {
> int res;
> res = dialog_inputbox(main_window,
> NULL, load_config_text,
> filename,
> dialog_input_result,
> - sizeof(dialog_input_result));
> + &dialog_input_result_len);
> switch (res) {
> case 0:
> if (!dialog_input_result[0])
> @@ -1424,14 +1424,13 @@ static void conf_load(void)
>
> static void conf_save(void)
> {
> - char dialog_input_result[256];
> while (1) {
> int res;
> res = dialog_inputbox(main_window,
> NULL, save_config_text,
> filename,
> dialog_input_result,
> - sizeof(dialog_input_result));
> + &dialog_input_result_len);
> switch (res) {
> case 0:
> if (!dialog_input_result[0])
> @@ -1488,6 +1487,15 @@ int main(int ac, char **av)
> single_menu_mode = 1;
> }
>
> + /* initially alloc 2048 bytes for dialog_input_result */
> + dialog_input_result_len = 2048;
> + dialog_input_result = malloc(dialog_input_result_len);
> + if (!dialog_input_result) {
> + fprintf(stderr, "Not enough memory for dialog_input_result(%d)\n",
> + dialog_input_result_len);
> + exit(1);
> + }
> +
I'm argue that we better have to be consistent with ourselves. If we
do not care about realloc(3) failing, we do not care either for that
one. There is already a whole bunch of unchecked malloc(3) calls in
kconfig, so one more will not change much.
> /* Initialize curses */
> initscr();
> /* set color theme */
> diff --git a/scripts/kconfig/nconf.gui.c b/scripts/kconfig/nconf.gui.c
> index 3ce2a7c..bc482ad 100644
> --- a/scripts/kconfig/nconf.gui.c
> +++ b/scripts/kconfig/nconf.gui.c
> @@ -356,7 +356,7 @@ int btn_dialog(WINDOW *main_window, const char *msg, int btn_num, ...)
>
> int dialog_inputbox(WINDOW *main_window,
> const char *title, const char *prompt,
> - const char *init, char *result, int result_len)
> + const char *init, char *result, int *result_len)
> {
> int prompt_lines = 0;
> int prompt_width = 0;
> @@ -368,6 +368,14 @@ int dialog_inputbox(WINDOW *main_window,
> int res = -1;
> int cursor_position = strlen(init);
>
> + if (strlen(init) > *result_len) {
> + do {
> + *result_len *= 2;
> + } while (strlen(init) > *result_len);
> + result = realloc(result, *result_len);
> + /* here didn't check result, if it's NULL,
> + just let it silently fail (SegFault) */
> + }
>
I do not think it's even worse to have the comment>
> /* find the widest line of msg: */
> prompt_lines = get_line_no(prompt);
> @@ -384,7 +392,7 @@ int dialog_inputbox(WINDOW *main_window,
> y = (LINES-(prompt_lines+4))/2;
> x = (COLS-(prompt_width+4))/2;
>
> - strncpy(result, init, result_len);
> + strncpy(result, init, *result_len);
>
> /* create the windows */
> win = newwin(prompt_lines+6, prompt_width+7, y, x);
> @@ -443,7 +451,7 @@ int dialog_inputbox(WINDOW *main_window,
> case KEY_UP:
> case KEY_RIGHT:
> if (cursor_position < len &&
> - cursor_position < min(result_len, prompt_width))
> + cursor_position < min(*result_len, prompt_width))
> cursor_position++;
> break;
> case KEY_DOWN:
> @@ -452,8 +460,15 @@ int dialog_inputbox(WINDOW *main_window,
> cursor_position--;
> break;
> default:
> - if ((isgraph(res) || isspace(res)) &&
> - len-2 < result_len) {
> + if ((isgraph(res) || isspace(res))) {
> + /* one for new char, one for '\0' */
> + if (len+2 > *result_len) {
> + do {
> + *result_len *= 2;
> + } while (len+2 > *result_len);
> + result = realloc(result, *result_len);
> + /* silently fail in the same way above */
> + }
likewise.
- Arnaud
> /* insert the char at the proper position */
> memmove(&result[cursor_position+1],
> &result[cursor_position],
> diff --git a/scripts/kconfig/nconf.h b/scripts/kconfig/nconf.h
> index 58fbda8..98b2c23 100644
> --- a/scripts/kconfig/nconf.h
> +++ b/scripts/kconfig/nconf.h
> @@ -89,7 +89,7 @@ void fill_window(WINDOW *win, const char *text);
> int btn_dialog(WINDOW *main_window, const char *msg, int btn_num, ...);
> int dialog_inputbox(WINDOW *main_window,
> const char *title, const char *prompt,
> - const char *init, char *result, int result_len);
> + const char *init, char *result, int *result_len);
> void refresh_all_windows(WINDOW *main_window);
> void show_scroll_win(WINDOW *main_window,
> const char *title,
> --
> 1.7.6
>
> --
> 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
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 3/5] scripts/kconfig/nconf: dynamically alloc dialog_input_result
2011-08-29 23:56 ` [PATCH 3/5] scripts/kconfig/nconf: dynamically alloc dialog_input_result Cheng Renquan
` (2 preceding siblings ...)
2011-08-30 3:25 ` Arnaud Lacombe
@ 2011-08-30 4:14 ` Arnaud Lacombe
2011-08-30 4:26 ` Arnaud Lacombe
3 siblings, 1 reply; 35+ messages in thread
From: Arnaud Lacombe @ 2011-08-30 4:14 UTC (permalink / raw)
To: Cheng Renquan
Cc: linux-kbuild, Sam Ravnborg, Michal Marek, Nir Tzachar,
Randy Dunlap, c.rq541
Hi,
On Mon, Aug 29, 2011 at 7:56 PM, Cheng Renquan <crquan@gmail.com> wrote:
> int dialog_inputbox(WINDOW *main_window,
> const char *title, const char *prompt,
> - const char *init, char *result, int result_len)
> + const char *init, char *result, int *result_len)
> {
This all logic will not work, you need to have the following prototype:
int dialog_inputbox(WINDOW *main_window,
const char *title, const char *prompt,
const char *init, char **result, int *result_len)
as realloc(3) will give you a new pointer.
- Arnaud
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 3/5] scripts/kconfig/nconf: dynamically alloc dialog_input_result
2011-08-30 4:14 ` Arnaud Lacombe
@ 2011-08-30 4:26 ` Arnaud Lacombe
0 siblings, 0 replies; 35+ messages in thread
From: Arnaud Lacombe @ 2011-08-30 4:26 UTC (permalink / raw)
To: Cheng Renquan
Cc: linux-kbuild, Sam Ravnborg, Michal Marek, Nir Tzachar,
Randy Dunlap, c.rq541
[-- Attachment #1: Type: text/plain, Size: 859 bytes --]
Hi,
On Tue, Aug 30, 2011 at 12:14 AM, Arnaud Lacombe <lacombar@gmail.com> wrote:
> Hi,
>
> On Mon, Aug 29, 2011 at 7:56 PM, Cheng Renquan <crquan@gmail.com> wrote:
>> int dialog_inputbox(WINDOW *main_window,
>> const char *title, const char *prompt,
>> - const char *init, char *result, int result_len)
>> + const char *init, char *result, int *result_len)
>> {
> This all logic will not work, you need to have the following prototype:
>
> int dialog_inputbox(WINDOW *main_window,
> const char *title, const char *prompt,
> const char *init, char **result, int *result_len)
>
> as realloc(3) will give you a new pointer.
>
Attached diff should do the job, this also avoid to have to deal with
`dialog_input_result' initialization.
- Arnaud
> - Arnaud
>
[-- Attachment #2: dialog_inputbox.diff --]
[-- Type: text/x-patch, Size: 3808 bytes --]
diff --git a/scripts/kconfig/nconf.c b/scripts/kconfig/nconf.c
index 18bfb34..53251f0 100644
--- a/scripts/kconfig/nconf.c
+++ b/scripts/kconfig/nconf.c
@@ -717,7 +717,7 @@ static void search_conf(void)
_("Search Configuration Parameter"),
_("Enter " CONFIG_ " (sub)string to search for "
"(with or without \"" CONFIG_ "\")"),
- "", dialog_input_result, &dialog_input_result_len);
+ "", &dialog_input_result, &dialog_input_result_len);
switch (dres) {
case 0:
break;
@@ -1384,7 +1384,7 @@ static void conf_string(struct menu *menu)
prompt ? _(prompt) : _("Main Menu"),
heading,
sym_get_string_value(menu->sym),
- dialog_input_result,
+ &dialog_input_result,
&dialog_input_result_len);
switch (res) {
case 0:
@@ -1410,7 +1410,7 @@ static void conf_load(void)
res = dialog_inputbox(main_window,
NULL, load_config_text,
filename,
- dialog_input_result,
+ &dialog_input_result,
&dialog_input_result_len);
switch (res) {
case 0:
@@ -1441,7 +1441,7 @@ static void conf_save(void)
res = dialog_inputbox(main_window,
NULL, save_config_text,
filename,
- dialog_input_result,
+ &dialog_input_result,
&dialog_input_result_len);
switch (res) {
case 0:
@@ -1508,15 +1508,6 @@ int main(int ac, char **av)
single_menu_mode = 1;
}
- /* initially alloc 2048 bytes for dialog_input_result */
- dialog_input_result_len = 2048;
- dialog_input_result = malloc(dialog_input_result_len);
- if (!dialog_input_result) {
- fprintf(stderr, "Not enough memory for dialog_input_result(%d)\n",
- dialog_input_result_len);
- exit(1);
- }
-
/* Initialize curses */
initscr();
/* set color theme */
diff --git a/scripts/kconfig/nconf.gui.c b/scripts/kconfig/nconf.gui.c
index 909c85f..8ae8d62 100644
--- a/scripts/kconfig/nconf.gui.c
+++ b/scripts/kconfig/nconf.gui.c
@@ -356,7 +356,7 @@ int btn_dialog(WINDOW *main_window, const char *msg, int btn_num, ...)
int dialog_inputbox(WINDOW *main_window,
const char *title, const char *prompt,
- const char *init, char *result, int *result_len)
+ const char *init, char **resultp, int *result_len)
{
int prompt_lines = 0;
int prompt_width = 0;
@@ -368,16 +368,16 @@ int dialog_inputbox(WINDOW *main_window,
int res = -1;
int cursor_position = strlen(init);
int cursor_form_win;
+ char *result = *resultp;
- if (strlen(init) > *result_len) {
- do {
- *result_len *= 2;
- } while (strlen(init) > *result_len);
+ if (strlen(init) + 1 > *result_len) {
+ *result_len = strlen(init) + 1;
result = realloc(result, *result_len);
- /* here didn't check result, if it's NULL,
- just let it silently fail (SegFault) */
+ *resultp = result;
}
@@ -480,11 +480,9 @@ int dialog_inputbox(WINDOW *main_window,
if ((isgraph(res) || isspace(res))) {
/* one for new char, one for '\0' */
if (len+2 > *result_len) {
- do {
- *result_len *= 2;
- } while (len+2 > *result_len);
+ *result_len = len+2;
result = realloc(result, *result_len);
- /* silently fail in the same way above */
+ *resultp = result;
}
/* insert the char at the proper position */
memmove(&result[cursor_position+1],
diff --git a/scripts/kconfig/nconf.h b/scripts/kconfig/nconf.h
index 98b2c23..0d52617 100644
--- a/scripts/kconfig/nconf.h
+++ b/scripts/kconfig/nconf.h
@@ -89,7 +89,7 @@ void fill_window(WINDOW *win, const char *text);
int btn_dialog(WINDOW *main_window, const char *msg, int btn_num, ...);
int dialog_inputbox(WINDOW *main_window,
const char *title, const char *prompt,
- const char *init, char *result, int *result_len);
+ const char *init, char **resultp, int *result_len);
void refresh_all_windows(WINDOW *main_window);
void show_scroll_win(WINDOW *main_window,
const char *title,
^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [PATCH 4/5] scripts/kconfig/nconf: fix editing long strings
2011-08-29 23:56 ` [PATCH 4/5] scripts/kconfig/nconf: fix editing long strings Cheng Renquan
2011-08-29 23:56 ` [PATCH 5/5] scripts/kconfig/nconf: add KEY_HOME / KEY_END for dialog_inputbox Cheng Renquan
@ 2011-08-30 4:59 ` Arnaud Lacombe
2011-08-31 8:39 ` Nir Tzachar
1 sibling, 1 reply; 35+ messages in thread
From: Arnaud Lacombe @ 2011-08-30 4:59 UTC (permalink / raw)
To: Cheng Renquan
Cc: linux-kbuild, Sam Ravnborg, Michal Marek, Nir Tzachar,
Randy Dunlap, c.rq541
Hi,
On Mon, Aug 29, 2011 at 7:56 PM, Cheng Renquan <crquan@gmail.com> wrote:
> The original dialog_inputbox doesn't work with longer than prompt_width
> strings, here fixed it in this way:
>
> 1) add variable cursor_form_win to record cursor of form_win,
> keep its value always between [0, prompt_width-1];
> keep the original cursor_position as cursor of the string result,
> for short strings, cursor_form_win is identical to cursor_position;
> for long strings, use (cursor_position-cursor_form_win) as begin offset
> to show part of the string in form_win;
>
> 2) whenever cursor of form_win is near (by 3 chars) to left or right edge
> of form_win, make a auto scroll by half prompt_width, to make this one
> line string editor more fun to use;
>
I am not a huge fan of this behavior, it seems a bit chaotic and
unnatural to me.
menuconfig's (partial) support of long string seem more stable. When
you erase a long string, the cursor move left. When you end up on the
edge, the window slide left fully, which let you see either a full
window again, or the beginning of the string. Unfortunately, you
cannot scroll within the string, but I would expect that when you
reach the right of the window, and continue typing, to see the window
moves right, without this "come back" effect.
Beside that, I agree, it's a huge improvement :)
- Arnaud
> 3) update len for later cursor_form_win correct calculation;
>
> Signed-off-by: Cheng Renquan <crquan@gmail.com>
> ---
> scripts/kconfig/nconf.gui.c | 43 +++++++++++++++++++++++++++++++++++++------
> 1 files changed, 37 insertions(+), 6 deletions(-)
>
> diff --git a/scripts/kconfig/nconf.gui.c b/scripts/kconfig/nconf.gui.c
> index bc482ad..62a41d1 100644
> --- a/scripts/kconfig/nconf.gui.c
> +++ b/scripts/kconfig/nconf.gui.c
> @@ -367,6 +367,7 @@ int dialog_inputbox(WINDOW *main_window,
> int i, x, y;
> int res = -1;
> int cursor_position = strlen(init);
> + int cursor_form_win;
>
> if (strlen(init) > *result_len) {
> do {
> @@ -413,7 +414,9 @@ int dialog_inputbox(WINDOW *main_window,
> fill_window(prompt_win, prompt);
>
> mvwprintw(form_win, 0, 0, "%*s", prompt_width, " ");
> - mvwprintw(form_win, 0, 0, "%s", result);
> + cursor_form_win = min(cursor_position, prompt_width-1);
> + mvwprintw(form_win, 0, 0, "%s",
> + result + cursor_position-cursor_form_win);
>
> /* create panels */
> panel = new_panel(win);
> @@ -439,6 +442,8 @@ int dialog_inputbox(WINDOW *main_window,
> &result[cursor_position],
> len-cursor_position+1);
> cursor_position--;
> + cursor_form_win--;
> + len--;
> }
> break;
> case KEY_DC:
> @@ -446,18 +451,22 @@ int dialog_inputbox(WINDOW *main_window,
> memmove(&result[cursor_position],
> &result[cursor_position+1],
> len-cursor_position+1);
> + len--;
> }
> break;
> case KEY_UP:
> case KEY_RIGHT:
> - if (cursor_position < len &&
> - cursor_position < min(*result_len, prompt_width))
> + if (cursor_position < len) {
> cursor_position++;
> + cursor_form_win++;
> + }
> break;
> case KEY_DOWN:
> case KEY_LEFT:
> - if (cursor_position > 0)
> + if (cursor_position > 0) {
> cursor_position--;
> + cursor_form_win--;
> + }
> break;
> default:
> if ((isgraph(res) || isspace(res))) {
> @@ -475,16 +484,38 @@ int dialog_inputbox(WINDOW *main_window,
> len-cursor_position+1);
> result[cursor_position] = res;
> cursor_position++;
> + cursor_form_win++;
> + len++;
> } else {
> mvprintw(0, 0, "unknown key: %d\n", res);
> }
> break;
> }
> + if (len <= prompt_width-1)
> + cursor_form_win = cursor_position;
> + else {
> + if (cursor_form_win <= 3)
> + cursor_form_win += prompt_width/2;
> + else if (cursor_form_win >= prompt_width-3)
> + cursor_form_win -= prompt_width/2;
> +
> + if (cursor_form_win < 0)
> + cursor_form_win = 0;
> + else if (cursor_form_win >= prompt_width-1)
> + cursor_form_win = prompt_width-1;
> +
> + if (cursor_form_win > cursor_position)
> + cursor_form_win = cursor_position;
> + if (cursor_form_win < (prompt_width-1) - (len-cursor_position))
> + cursor_form_win = (prompt_width-1) - (len-cursor_position);
> + }
> +
> wmove(form_win, 0, 0);
> wclrtoeol(form_win);
> mvwprintw(form_win, 0, 0, "%*s", prompt_width, " ");
> - mvwprintw(form_win, 0, 0, "%s", result);
> - wmove(form_win, 0, cursor_position);
> + mvwprintw(form_win, 0, 0, "%s",
> + result + cursor_position-cursor_form_win);
> + wmove(form_win, 0, cursor_form_win);
> touchwin(win);
> refresh_all_windows(main_window);
>
> --
> 1.7.6
>
> --
> 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
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 0/6] nconf bug fixes and U/E improvements
2011-08-29 9:09 [RFC] nconf bug fixes and improvements Cheng Renquan
` (2 preceding siblings ...)
2011-08-29 23:56 ` [PATCH 1/5] scripts/kconfig/nconf: fix typo: unknow => unknown Cheng Renquan
@ 2011-08-31 7:46 ` Cheng Renquan
2011-08-31 7:46 ` [PATCH V2 1/6] scripts/kconfig/nconf: fix typo: unknow => unknown Cheng Renquan
2011-08-31 12:36 ` [PATCH 0/6] nconf bug fixes and U/E improvements Cheng Renquan
2011-09-01 17:52 ` [PATCH V3 1/5] scripts/kconfig/nconf: fix typo: unknow => unknown crquan
4 siblings, 2 replies; 35+ messages in thread
From: Cheng Renquan @ 2011-08-31 7:46 UTC (permalink / raw)
To: linux-kbuild, Arnaud Lacombe
Cc: Sam Ravnborg, Michal Marek, Nir Tzachar, Randy Dunlap, c.rq541
*** BLURB HERE ***
Cheng Renquan (6):
scripts/kconfig/nconf: fix typo: unknow => unknown
scripts/kconfig/nconf: fix memmove's length arg
scripts/kconfig/nconf: dynamically alloc dialog_input_result
Thanks Arnaud pointing out realloc issue in previous edition;
scripts/kconfig/nconf: fix editing long strings
scripts/kconfig/nconf: add KEY_HOME / KEY_END for dialog_inputbox
The nconfig even has KEY_UP as KEY_RIGHT and KEY_DOWN as KEY_LEFT,
that I think more unnatural, better remove KEY_UP/KEY_DOWN in such
a one line editor, or for future possible text area editor;
scripts/kconfig/nconf: trunc too long string display in menu
scripts/kconfig/nconf.c | 29 ++++++++++-------
scripts/kconfig/nconf.gui.c | 73 ++++++++++++++++++++++++++++++++++++-------
scripts/kconfig/nconf.h | 2 +-
3 files changed, 79 insertions(+), 25 deletions(-)
--
1.7.6
Cheng Renquan (???)
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH V2 1/6] scripts/kconfig/nconf: fix typo: unknow => unknown
2011-08-31 7:46 ` [PATCH 0/6] nconf bug fixes and U/E improvements Cheng Renquan
@ 2011-08-31 7:46 ` Cheng Renquan
2011-08-31 7:46 ` [PATCH V2 2/6] scripts/kconfig/nconf: fix memmove's length arg Cheng Renquan
2011-08-31 12:36 ` [PATCH 0/6] nconf bug fixes and U/E improvements Cheng Renquan
1 sibling, 1 reply; 35+ messages in thread
From: Cheng Renquan @ 2011-08-31 7:46 UTC (permalink / raw)
To: linux-kbuild, Arnaud Lacombe
Cc: Sam Ravnborg, Michal Marek, Nir Tzachar, Randy Dunlap, c.rq541
Signed-off-by: Cheng Renquan <crquan@gmail.com>
Acked-by: Arnaud Lacombe <lacombar@gmail.com>
---
scripts/kconfig/nconf.gui.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/scripts/kconfig/nconf.gui.c b/scripts/kconfig/nconf.gui.c
index f8137b3..d3af04e 100644
--- a/scripts/kconfig/nconf.gui.c
+++ b/scripts/kconfig/nconf.gui.c
@@ -461,7 +461,7 @@ int dialog_inputbox(WINDOW *main_window,
result[cursor_position] = res;
cursor_position++;
} else {
- mvprintw(0, 0, "unknow key: %d\n", res);
+ mvprintw(0, 0, "unknown key: %d\n", res);
}
break;
}
--
1.7.6
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH V2 2/6] scripts/kconfig/nconf: fix memmove's length arg
2011-08-31 7:46 ` [PATCH V2 1/6] scripts/kconfig/nconf: fix typo: unknow => unknown Cheng Renquan
@ 2011-08-31 7:46 ` Cheng Renquan
2011-08-31 7:46 ` [PATCH V2 3/6] scripts/kconfig/nconf: dynamically alloc dialog_input_result Cheng Renquan
2011-08-31 8:30 ` [PATCH V2 2/6] scripts/kconfig/nconf: fix memmove's length arg Nir Tzachar
0 siblings, 2 replies; 35+ messages in thread
From: Cheng Renquan @ 2011-08-31 7:46 UTC (permalink / raw)
To: linux-kbuild, Arnaud Lacombe
Cc: Sam Ravnborg, Michal Marek, Nir Tzachar, Randy Dunlap, c.rq541
In case KEY_BACKSPACE / KEY_DC to delete a char, it memmove only
(len-cursor_position+1) bytes;
the default case is to insert a char, it should also memmove exactly
(len-cursor_position+1) bytes;
the original use of (len+1) is wrong and may access following memory
that doesn't belong to result, may cause SegFault in theory;
case KEY_BACKSPACE:
if (cursor_position > 0) {
memmove(&result[cursor_position-1],
&result[cursor_position],
len-cursor_position+1);
cursor_position--;
}
break;
case KEY_DC:
if (cursor_position >= 0 && cursor_position < len) {
memmove(&result[cursor_position],
&result[cursor_position+1],
len-cursor_position+1);
}
break;
default:
if ((isgraph(res) || isspace(res)) &&
len-2 < result_len) {
/* insert the char at the proper position */
memmove(&result[cursor_position+1],
&result[cursor_position],
len-cursor_position+1);
result[cursor_position] = res;
cursor_position++;
}
Signed-off-by: Cheng Renquan <crquan@gmail.com>
---
scripts/kconfig/nconf.gui.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/scripts/kconfig/nconf.gui.c b/scripts/kconfig/nconf.gui.c
index d3af04e..3ce2a7c 100644
--- a/scripts/kconfig/nconf.gui.c
+++ b/scripts/kconfig/nconf.gui.c
@@ -457,7 +457,7 @@ int dialog_inputbox(WINDOW *main_window,
/* insert the char at the proper position */
memmove(&result[cursor_position+1],
&result[cursor_position],
- len+1);
+ len-cursor_position+1);
result[cursor_position] = res;
cursor_position++;
} else {
--
1.7.6
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH V2 3/6] scripts/kconfig/nconf: dynamically alloc dialog_input_result
2011-08-31 7:46 ` [PATCH V2 2/6] scripts/kconfig/nconf: fix memmove's length arg Cheng Renquan
@ 2011-08-31 7:46 ` Cheng Renquan
2011-08-31 7:46 ` [PATCH V2 4/6] scripts/kconfig/nconf: fix editing long strings Cheng Renquan
2011-08-31 8:30 ` [PATCH V2 2/6] scripts/kconfig/nconf: fix memmove's length arg Nir Tzachar
1 sibling, 1 reply; 35+ messages in thread
From: Cheng Renquan @ 2011-08-31 7:46 UTC (permalink / raw)
To: linux-kbuild, Arnaud Lacombe
Cc: Sam Ravnborg, Michal Marek, Nir Tzachar, Randy Dunlap, c.rq541
To support unlimited length string config items;
No check for realloc return value keeps code simple, and to be
consistent with other existing unchecked malloc in kconfig.
Signed-off-by: Cheng Renquan <crquan@gmail.com>
Signed-off-by: Arnaud Lacombe <lacombar@gmail.com>
---
scripts/kconfig/nconf.c | 21 ++++++++++-----------
scripts/kconfig/nconf.gui.c | 20 +++++++++++++++-----
scripts/kconfig/nconf.h | 2 +-
3 files changed, 26 insertions(+), 17 deletions(-)
diff --git a/scripts/kconfig/nconf.c b/scripts/kconfig/nconf.c
index 39ca1f1..4248759 100644
--- a/scripts/kconfig/nconf.c
+++ b/scripts/kconfig/nconf.c
@@ -280,6 +280,9 @@ static int global_exit;
/* the currently selected button */
const char *current_instructions = menu_instructions;
+static char *dialog_input_result;
+static int dialog_input_result_len;
+
static void conf(struct menu *menu);
static void conf_choice(struct menu *menu);
static void conf_string(struct menu *menu);
@@ -695,7 +698,6 @@ static void search_conf(void)
{
struct symbol **sym_arr;
struct gstr res;
- char dialog_input_result[100];
char *dialog_input;
int dres;
again:
@@ -703,7 +705,7 @@ again:
_("Search Configuration Parameter"),
_("Enter " CONFIG_ " (sub)string to search for "
"(with or without \"" CONFIG_ "\")"),
- "", dialog_input_result, 99);
+ "", &dialog_input_result, &dialog_input_result_len);
switch (dres) {
case 0:
break;
@@ -1348,7 +1350,6 @@ static void conf_choice(struct menu *menu)
static void conf_string(struct menu *menu)
{
const char *prompt = menu_get_prompt(menu);
- char dialog_input_result[256];
while (1) {
int res;
@@ -1371,8 +1372,8 @@ static void conf_string(struct menu *menu)
prompt ? _(prompt) : _("Main Menu"),
heading,
sym_get_string_value(menu->sym),
- dialog_input_result,
- sizeof(dialog_input_result));
+ &dialog_input_result,
+ &dialog_input_result_len);
switch (res) {
case 0:
if (sym_set_string_value(menu->sym,
@@ -1392,14 +1393,13 @@ static void conf_string(struct menu *menu)
static void conf_load(void)
{
- char dialog_input_result[256];
while (1) {
int res;
res = dialog_inputbox(main_window,
NULL, load_config_text,
filename,
- dialog_input_result,
- sizeof(dialog_input_result));
+ &dialog_input_result,
+ &dialog_input_result_len);
switch (res) {
case 0:
if (!dialog_input_result[0])
@@ -1424,14 +1424,13 @@ static void conf_load(void)
static void conf_save(void)
{
- char dialog_input_result[256];
while (1) {
int res;
res = dialog_inputbox(main_window,
NULL, save_config_text,
filename,
- dialog_input_result,
- sizeof(dialog_input_result));
+ &dialog_input_result,
+ &dialog_input_result_len);
switch (res) {
case 0:
if (!dialog_input_result[0])
diff --git a/scripts/kconfig/nconf.gui.c b/scripts/kconfig/nconf.gui.c
index 3ce2a7c..d64bc1c 100644
--- a/scripts/kconfig/nconf.gui.c
+++ b/scripts/kconfig/nconf.gui.c
@@ -356,7 +356,7 @@ int btn_dialog(WINDOW *main_window, const char *msg, int btn_num, ...)
int dialog_inputbox(WINDOW *main_window,
const char *title, const char *prompt,
- const char *init, char *result, int result_len)
+ const char *init, char **resultp, int *result_len)
{
int prompt_lines = 0;
int prompt_width = 0;
@@ -367,7 +367,12 @@ int dialog_inputbox(WINDOW *main_window,
int i, x, y;
int res = -1;
int cursor_position = strlen(init);
+ char *result = *resultp;
+ if (strlen(init)+1 > *result_len) {
+ *result_len = strlen(init)+1;
+ *resultp = result = realloc(result, *result_len);
+ }
/* find the widest line of msg: */
prompt_lines = get_line_no(prompt);
@@ -384,7 +389,7 @@ int dialog_inputbox(WINDOW *main_window,
y = (LINES-(prompt_lines+4))/2;
x = (COLS-(prompt_width+4))/2;
- strncpy(result, init, result_len);
+ strncpy(result, init, *result_len);
/* create the windows */
win = newwin(prompt_lines+6, prompt_width+7, y, x);
@@ -443,7 +448,7 @@ int dialog_inputbox(WINDOW *main_window,
case KEY_UP:
case KEY_RIGHT:
if (cursor_position < len &&
- cursor_position < min(result_len, prompt_width))
+ cursor_position < min(*result_len, prompt_width))
cursor_position++;
break;
case KEY_DOWN:
@@ -452,8 +457,13 @@ int dialog_inputbox(WINDOW *main_window,
cursor_position--;
break;
default:
- if ((isgraph(res) || isspace(res)) &&
- len-2 < result_len) {
+ if ((isgraph(res) || isspace(res))) {
+ /* one for new char, one for '\0' */
+ if (len+2 > *result_len) {
+ *result_len = len+2;
+ *resultp = result = realloc(result,
+ *result_len);
+ }
/* insert the char at the proper position */
memmove(&result[cursor_position+1],
&result[cursor_position],
diff --git a/scripts/kconfig/nconf.h b/scripts/kconfig/nconf.h
index 58fbda8..0d52617 100644
--- a/scripts/kconfig/nconf.h
+++ b/scripts/kconfig/nconf.h
@@ -89,7 +89,7 @@ void fill_window(WINDOW *win, const char *text);
int btn_dialog(WINDOW *main_window, const char *msg, int btn_num, ...);
int dialog_inputbox(WINDOW *main_window,
const char *title, const char *prompt,
- const char *init, char *result, int result_len);
+ const char *init, char **resultp, int *result_len);
void refresh_all_windows(WINDOW *main_window);
void show_scroll_win(WINDOW *main_window,
const char *title,
--
1.7.6
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH V2 4/6] scripts/kconfig/nconf: fix editing long strings
2011-08-31 7:46 ` [PATCH V2 3/6] scripts/kconfig/nconf: dynamically alloc dialog_input_result Cheng Renquan
@ 2011-08-31 7:46 ` Cheng Renquan
2011-08-31 7:46 ` [PATCH V2 5/6] scripts/kconfig/nconf: add KEY_HOME / KEY_END for dialog_inputbox Cheng Renquan
0 siblings, 1 reply; 35+ messages in thread
From: Cheng Renquan @ 2011-08-31 7:46 UTC (permalink / raw)
To: linux-kbuild, Arnaud Lacombe
Cc: Sam Ravnborg, Michal Marek, Nir Tzachar, Randy Dunlap, c.rq541
The original dialog_inputbox doesn't work with longer than prompt_width
strings, here fixed it in this way:
1) add variable cursor_form_win to record cursor of form_win,
keep its value always between [0, prompt_width-1];
keep the original cursor_position as cursor of the string result,
for short strings, cursor_form_win is identical to cursor_position;
for long strings, use (cursor_position-cursor_form_win) as begin offset
to show part of the string in form_win;
2) whenever cursor of form_win is near (by 3 chars) to left or right edge
of form_win, make a auto scroll by half prompt_width, to make this one
line string editor more fun to use;
3) update len for later cursor_form_win correct calculation;
Signed-off-by: Cheng Renquan <crquan@gmail.com>
---
scripts/kconfig/nconf.gui.c | 43 +++++++++++++++++++++++++++++++++++++------
1 files changed, 37 insertions(+), 6 deletions(-)
diff --git a/scripts/kconfig/nconf.gui.c b/scripts/kconfig/nconf.gui.c
index d64bc1c..a236ae7 100644
--- a/scripts/kconfig/nconf.gui.c
+++ b/scripts/kconfig/nconf.gui.c
@@ -367,6 +367,7 @@ int dialog_inputbox(WINDOW *main_window,
int i, x, y;
int res = -1;
int cursor_position = strlen(init);
+ int cursor_form_win;
char *result = *resultp;
if (strlen(init)+1 > *result_len) {
@@ -410,7 +411,9 @@ int dialog_inputbox(WINDOW *main_window,
fill_window(prompt_win, prompt);
mvwprintw(form_win, 0, 0, "%*s", prompt_width, " ");
- mvwprintw(form_win, 0, 0, "%s", result);
+ cursor_form_win = min(cursor_position, prompt_width-1);
+ mvwprintw(form_win, 0, 0, "%s",
+ result + cursor_position-cursor_form_win);
/* create panels */
panel = new_panel(win);
@@ -436,6 +439,8 @@ int dialog_inputbox(WINDOW *main_window,
&result[cursor_position],
len-cursor_position+1);
cursor_position--;
+ cursor_form_win--;
+ len--;
}
break;
case KEY_DC:
@@ -443,18 +448,22 @@ int dialog_inputbox(WINDOW *main_window,
memmove(&result[cursor_position],
&result[cursor_position+1],
len-cursor_position+1);
+ len--;
}
break;
case KEY_UP:
case KEY_RIGHT:
- if (cursor_position < len &&
- cursor_position < min(*result_len, prompt_width))
+ if (cursor_position < len) {
cursor_position++;
+ cursor_form_win++;
+ }
break;
case KEY_DOWN:
case KEY_LEFT:
- if (cursor_position > 0)
+ if (cursor_position > 0) {
cursor_position--;
+ cursor_form_win--;
+ }
break;
default:
if ((isgraph(res) || isspace(res))) {
@@ -470,16 +479,38 @@ int dialog_inputbox(WINDOW *main_window,
len-cursor_position+1);
result[cursor_position] = res;
cursor_position++;
+ cursor_form_win++;
+ len++;
} else {
mvprintw(0, 0, "unknown key: %d\n", res);
}
break;
}
+ if (len <= prompt_width-1)
+ cursor_form_win = cursor_position;
+ else {
+ if (cursor_form_win <= 3)
+ cursor_form_win += prompt_width/2;
+ else if (cursor_form_win >= prompt_width-3)
+ cursor_form_win -= prompt_width/2;
+
+ if (cursor_form_win < 0)
+ cursor_form_win = 0;
+ else if (cursor_form_win >= prompt_width-1)
+ cursor_form_win = prompt_width-1;
+
+ if (cursor_form_win > cursor_position)
+ cursor_form_win = cursor_position;
+ if (cursor_form_win < (prompt_width-1) - (len-cursor_position))
+ cursor_form_win = (prompt_width-1) - (len-cursor_position);
+ }
+
wmove(form_win, 0, 0);
wclrtoeol(form_win);
mvwprintw(form_win, 0, 0, "%*s", prompt_width, " ");
- mvwprintw(form_win, 0, 0, "%s", result);
- wmove(form_win, 0, cursor_position);
+ mvwprintw(form_win, 0, 0, "%s",
+ result + cursor_position-cursor_form_win);
+ wmove(form_win, 0, cursor_form_win);
touchwin(win);
refresh_all_windows(main_window);
--
1.7.6
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH V2 5/6] scripts/kconfig/nconf: add KEY_HOME / KEY_END for dialog_inputbox
2011-08-31 7:46 ` [PATCH V2 4/6] scripts/kconfig/nconf: fix editing long strings Cheng Renquan
@ 2011-08-31 7:46 ` Cheng Renquan
2011-08-31 7:46 ` [PATCH V2 6/6] scripts/kconfig/nconf: trunc too long string display in menu Cheng Renquan
2011-08-31 8:42 ` [PATCH V2 5/6] scripts/kconfig/nconf: add KEY_HOME / KEY_END for dialog_inputbox Nir Tzachar
0 siblings, 2 replies; 35+ messages in thread
From: Cheng Renquan @ 2011-08-31 7:46 UTC (permalink / raw)
To: linux-kbuild, Arnaud Lacombe
Cc: Sam Ravnborg, Michal Marek, Nir Tzachar, Randy Dunlap, c.rq541
to make it easier to locate begin/end when editing long strings;
Signed-off-by: Cheng Renquan <crquan@gmail.com>
---
scripts/kconfig/nconf.gui.c | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/scripts/kconfig/nconf.gui.c b/scripts/kconfig/nconf.gui.c
index a236ae7..1d73897 100644
--- a/scripts/kconfig/nconf.gui.c
+++ b/scripts/kconfig/nconf.gui.c
@@ -465,6 +465,14 @@ int dialog_inputbox(WINDOW *main_window,
cursor_form_win--;
}
break;
+ case KEY_HOME:
+ cursor_position = 0;
+ cursor_form_win = 0;
+ break;
+ case KEY_END:
+ cursor_position = len;
+ cursor_form_win = min(cursor_position, prompt_width-1);
+ break;
default:
if ((isgraph(res) || isspace(res))) {
/* one for new char, one for '\0' */
--
1.7.6
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH V2 6/6] scripts/kconfig/nconf: trunc too long string display in menu
2011-08-31 7:46 ` [PATCH V2 5/6] scripts/kconfig/nconf: add KEY_HOME / KEY_END for dialog_inputbox Cheng Renquan
@ 2011-08-31 7:46 ` Cheng Renquan
2011-08-31 8:53 ` Nir Tzachar
2011-08-31 8:42 ` [PATCH V2 5/6] scripts/kconfig/nconf: add KEY_HOME / KEY_END for dialog_inputbox Nir Tzachar
1 sibling, 1 reply; 35+ messages in thread
From: Cheng Renquan @ 2011-08-31 7:46 UTC (permalink / raw)
To: linux-kbuild, Arnaud Lacombe
Cc: Sam Ravnborg, Michal Marek, Nir Tzachar, Randy Dunlap, c.rq541
When the string is too long, only show the first COLS/2 bytes wide,
to make sure there is some space to show the menu prompt.
Signed-off-by: Cheng Renquan <crquan@gmail.com>
---
scripts/kconfig/nconf.c | 8 +++++++-
1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/scripts/kconfig/nconf.c b/scripts/kconfig/nconf.c
index 4248759..4997ebf 100644
--- a/scripts/kconfig/nconf.c
+++ b/scripts/kconfig/nconf.c
@@ -887,8 +887,14 @@ static void build_conf(struct menu *menu)
break;
default:
tmp = 2 + strlen(sym_get_string_value(sym));
- item_make(menu, 's', " (%s)",
+ if (tmp < COLS/2)
+ item_make(menu, 's', " (%s)",
sym_get_string_value(sym));
+ else {
+ char *s = strndupa(sym_get_string_value(sym), COLS/2);
+ strcpy(s + COLS/2 - 5, " ...");
+ item_make(menu, 's', " (%s)", s);
+ }
tmp = indent - tmp + 4;
if (tmp < 0)
tmp = 0;
--
1.7.6
^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [PATCH V2 2/6] scripts/kconfig/nconf: fix memmove's length arg
2011-08-31 7:46 ` [PATCH V2 2/6] scripts/kconfig/nconf: fix memmove's length arg Cheng Renquan
2011-08-31 7:46 ` [PATCH V2 3/6] scripts/kconfig/nconf: dynamically alloc dialog_input_result Cheng Renquan
@ 2011-08-31 8:30 ` Nir Tzachar
1 sibling, 0 replies; 35+ messages in thread
From: Nir Tzachar @ 2011-08-31 8:30 UTC (permalink / raw)
To: Cheng Renquan
Cc: linux-kbuild, Arnaud Lacombe, Sam Ravnborg, Michal Marek,
Randy Dunlap, c.rq541
On Wed, Aug 31, 2011 at 10:46 AM, Cheng Renquan <crquan@gmail.com> wrote:
> In case KEY_BACKSPACE / KEY_DC to delete a char, it memmove only
> (len-cursor_position+1) bytes;
> the default case is to insert a char, it should also memmove exactly
> (len-cursor_position+1) bytes;
>
> the original use of (len+1) is wrong and may access following memory
> that doesn't belong to result, may cause SegFault in theory;
>
> case KEY_BACKSPACE:
> if (cursor_position > 0) {
> memmove(&result[cursor_position-1],
> &result[cursor_position],
> len-cursor_position+1);
> cursor_position--;
> }
> break;
> case KEY_DC:
> if (cursor_position >= 0 && cursor_position < len) {
> memmove(&result[cursor_position],
> &result[cursor_position+1],
> len-cursor_position+1);
> }
> break;
> default:
> if ((isgraph(res) || isspace(res)) &&
> len-2 < result_len) {
> /* insert the char at the proper position */
> memmove(&result[cursor_position+1],
> &result[cursor_position],
> len-cursor_position+1);
> result[cursor_position] = res;
> cursor_position++;
> }
>
> Signed-off-by: Cheng Renquan <crquan@gmail.com>
> ---
> scripts/kconfig/nconf.gui.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/scripts/kconfig/nconf.gui.c b/scripts/kconfig/nconf.gui.c
> index d3af04e..3ce2a7c 100644
> --- a/scripts/kconfig/nconf.gui.c
> +++ b/scripts/kconfig/nconf.gui.c
> @@ -457,7 +457,7 @@ int dialog_inputbox(WINDOW *main_window,
> /* insert the char at the proper position */
> memmove(&result[cursor_position+1],
> &result[cursor_position],
> - len+1);
> + len-cursor_position+1);
> result[cursor_position] = res;
> cursor_position++;
> } else {
> --
> 1.7.6
>
>
Acked-by: Nir Tzachar <nir.tzachar@gmail.com>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 4/5] scripts/kconfig/nconf: fix editing long strings
2011-08-30 4:59 ` [PATCH 4/5] scripts/kconfig/nconf: fix editing long strings Arnaud Lacombe
@ 2011-08-31 8:39 ` Nir Tzachar
0 siblings, 0 replies; 35+ messages in thread
From: Nir Tzachar @ 2011-08-31 8:39 UTC (permalink / raw)
To: Arnaud Lacombe
Cc: Cheng Renquan, linux-kbuild, Sam Ravnborg, Michal Marek,
Randy Dunlap, c.rq541
Hello.
On Tue, Aug 30, 2011 at 7:59 AM, Arnaud Lacombe <lacombar@gmail.com> wrote:
> Hi,
>
> On Mon, Aug 29, 2011 at 7:56 PM, Cheng Renquan <crquan@gmail.com> wrote:
>> The original dialog_inputbox doesn't work with longer than prompt_width
>> strings, here fixed it in this way:
>>
>> 1) add variable cursor_form_win to record cursor of form_win,
>> keep its value always between [0, prompt_width-1];
>> keep the original cursor_position as cursor of the string result,
>> for short strings, cursor_form_win is identical to cursor_position;
>> for long strings, use (cursor_position-cursor_form_win) as begin offset
>> to show part of the string in form_win;
>>
>
>> 2) whenever cursor of form_win is near (by 3 chars) to left or right edge
>> of form_win, make a auto scroll by half prompt_width, to make this one
>> line string editor more fun to use;
>>
> I am not a huge fan of this behavior, it seems a bit chaotic and
> unnatural to me.
I agree with Arnaud. Jumping the line is chaotic. I believe that
scrolling the string view one character left (or right)-wise is a
better solution.
> menuconfig's (partial) support of long string seem more stable. When
> you erase a long string, the cursor move left. When you end up on the
> edge, the window slide left fully, which let you see either a full
> window again, or the beginning of the string. Unfortunately, you
> cannot scroll within the string, but I would expect that when you
> reach the right of the window, and continue typing, to see the window
> moves right, without this "come back" effect.
>
> Beside that, I agree, it's a huge improvement :)
>
> - Arnaud
>
>> 3) update len for later cursor_form_win correct calculation;
>>
>> Signed-off-by: Cheng Renquan <crquan@gmail.com>
>> ---
>> scripts/kconfig/nconf.gui.c | 43 +++++++++++++++++++++++++++++++++++++------
>> 1 files changed, 37 insertions(+), 6 deletions(-)
>>
>> diff --git a/scripts/kconfig/nconf.gui.c b/scripts/kconfig/nconf.gui.c
>> index bc482ad..62a41d1 100644
>> --- a/scripts/kconfig/nconf.gui.c
>> +++ b/scripts/kconfig/nconf.gui.c
>> @@ -367,6 +367,7 @@ int dialog_inputbox(WINDOW *main_window,
>> int i, x, y;
>> int res = -1;
>> int cursor_position = strlen(init);
>> + int cursor_form_win;
>>
>> if (strlen(init) > *result_len) {
>> do {
>> @@ -413,7 +414,9 @@ int dialog_inputbox(WINDOW *main_window,
>> fill_window(prompt_win, prompt);
>>
>> mvwprintw(form_win, 0, 0, "%*s", prompt_width, " ");
>> - mvwprintw(form_win, 0, 0, "%s", result);
>> + cursor_form_win = min(cursor_position, prompt_width-1);
>> + mvwprintw(form_win, 0, 0, "%s",
>> + result + cursor_position-cursor_form_win);
>>
>> /* create panels */
>> panel = new_panel(win);
>> @@ -439,6 +442,8 @@ int dialog_inputbox(WINDOW *main_window,
>> &result[cursor_position],
>> len-cursor_position+1);
>> cursor_position--;
>> + cursor_form_win--;
>> + len--;
>> }
>> break;
>> case KEY_DC:
>> @@ -446,18 +451,22 @@ int dialog_inputbox(WINDOW *main_window,
>> memmove(&result[cursor_position],
>> &result[cursor_position+1],
>> len-cursor_position+1);
>> + len--;
>> }
>> break;
>> case KEY_UP:
>> case KEY_RIGHT:
>> - if (cursor_position < len &&
>> - cursor_position < min(*result_len, prompt_width))
>> + if (cursor_position < len) {
>> cursor_position++;
>> + cursor_form_win++;
>> + }
>> break;
>> case KEY_DOWN:
>> case KEY_LEFT:
>> - if (cursor_position > 0)
>> + if (cursor_position > 0) {
>> cursor_position--;
>> + cursor_form_win--;
>> + }
>> break;
>> default:
>> if ((isgraph(res) || isspace(res))) {
>> @@ -475,16 +484,38 @@ int dialog_inputbox(WINDOW *main_window,
>> len-cursor_position+1);
>> result[cursor_position] = res;
>> cursor_position++;
>> + cursor_form_win++;
>> + len++;
>> } else {
>> mvprintw(0, 0, "unknown key: %d\n", res);
>> }
>> break;
>> }
>> + if (len <= prompt_width-1)
>> + cursor_form_win = cursor_position;
>> + else {
>> + if (cursor_form_win <= 3)
>> + cursor_form_win += prompt_width/2;
>> + else if (cursor_form_win >= prompt_width-3)
>> + cursor_form_win -= prompt_width/2;
>> +
>> + if (cursor_form_win < 0)
>> + cursor_form_win = 0;
>> + else if (cursor_form_win >= prompt_width-1)
>> + cursor_form_win = prompt_width-1;
>> +
>> + if (cursor_form_win > cursor_position)
>> + cursor_form_win = cursor_position;
>> + if (cursor_form_win < (prompt_width-1) - (len-cursor_position))
>> + cursor_form_win = (prompt_width-1) - (len-cursor_position);
>> + }
>> +
>> wmove(form_win, 0, 0);
>> wclrtoeol(form_win);
>> mvwprintw(form_win, 0, 0, "%*s", prompt_width, " ");
>> - mvwprintw(form_win, 0, 0, "%s", result);
>> - wmove(form_win, 0, cursor_position);
>> + mvwprintw(form_win, 0, 0, "%s",
>> + result + cursor_position-cursor_form_win);
>> + wmove(form_win, 0, cursor_form_win);
>> touchwin(win);
>> refresh_all_windows(main_window);
>>
>> --
>> 1.7.6
>>
>> --
>> 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
>>
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH V2 5/6] scripts/kconfig/nconf: add KEY_HOME / KEY_END for dialog_inputbox
2011-08-31 7:46 ` [PATCH V2 5/6] scripts/kconfig/nconf: add KEY_HOME / KEY_END for dialog_inputbox Cheng Renquan
2011-08-31 7:46 ` [PATCH V2 6/6] scripts/kconfig/nconf: trunc too long string display in menu Cheng Renquan
@ 2011-08-31 8:42 ` Nir Tzachar
1 sibling, 0 replies; 35+ messages in thread
From: Nir Tzachar @ 2011-08-31 8:42 UTC (permalink / raw)
To: Cheng Renquan
Cc: linux-kbuild, Arnaud Lacombe, Sam Ravnborg, Michal Marek,
Randy Dunlap, c.rq541
On Wed, Aug 31, 2011 at 10:46 AM, Cheng Renquan <crquan@gmail.com> wrote:
> to make it easier to locate begin/end when editing long strings;
>
> Signed-off-by: Cheng Renquan <crquan@gmail.com>
> ---
> scripts/kconfig/nconf.gui.c | 8 ++++++++
> 1 files changed, 8 insertions(+), 0 deletions(-)
>
> diff --git a/scripts/kconfig/nconf.gui.c b/scripts/kconfig/nconf.gui.c
> index a236ae7..1d73897 100644
> --- a/scripts/kconfig/nconf.gui.c
> +++ b/scripts/kconfig/nconf.gui.c
> @@ -465,6 +465,14 @@ int dialog_inputbox(WINDOW *main_window,
> cursor_form_win--;
> }
> break;
> + case KEY_HOME:
> + cursor_position = 0;
> + cursor_form_win = 0;
> + break;
> + case KEY_END:
> + cursor_position = len;
> + cursor_form_win = min(cursor_position, prompt_width-1);
> + break;
> default:
> if ((isgraph(res) || isspace(res))) {
> /* one for new char, one for '\0' */
> --
> 1.7.6
>
>
Acked By: Nir Tzachar <nir.tzachar@gmail.com>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH V2 6/6] scripts/kconfig/nconf: trunc too long string display in menu
2011-08-31 7:46 ` [PATCH V2 6/6] scripts/kconfig/nconf: trunc too long string display in menu Cheng Renquan
@ 2011-08-31 8:53 ` Nir Tzachar
2011-08-31 13:04 ` Cheng Renquan
0 siblings, 1 reply; 35+ messages in thread
From: Nir Tzachar @ 2011-08-31 8:53 UTC (permalink / raw)
To: Cheng Renquan
Cc: linux-kbuild, Arnaud Lacombe, Sam Ravnborg, Michal Marek,
Randy Dunlap, c.rq541
Hi.
On Wed, Aug 31, 2011 at 10:46 AM, Cheng Renquan <crquan@gmail.com> wrote:
> When the string is too long, only show the first COLS/2 bytes wide,
> to make sure there is some space to show the menu prompt.
>
> Signed-off-by: Cheng Renquan <crquan@gmail.com>
> ---
> scripts/kconfig/nconf.c | 8 +++++++-
> 1 files changed, 7 insertions(+), 1 deletions(-)
>
> diff --git a/scripts/kconfig/nconf.c b/scripts/kconfig/nconf.c
> index 4248759..4997ebf 100644
> --- a/scripts/kconfig/nconf.c
> +++ b/scripts/kconfig/nconf.c
> @@ -887,8 +887,14 @@ static void build_conf(struct menu *menu)
> break;
> default:
> tmp = 2 + strlen(sym_get_string_value(sym));
> - item_make(menu, 's', " (%s)",
> + if (tmp < COLS/2)
> + item_make(menu, 's', " (%s)",
> sym_get_string_value(sym));
> + else {
> + char *s = strndupa(sym_get_string_value(sym), COLS/2);
> + strcpy(s + COLS/2 - 5, " ...");
> + item_make(menu, 's', " (%s)", s);
> + }
> tmp = indent - tmp + 4;
> if (tmp < 0)
> tmp = 0;
> --
> 1.7.6
>
>
Nack.
I see several problems with this approach:
1) I do not like the use of strdupa.
2) I think it would be better to put this behavior in item_make(), as
it will then be applied to all items, in all code paths. However, I a
not sure that using COLS/2 is the best approach. Moreover, I think
that this should be implemented inside the ncurses menu library and
not worked around. Care to create a patch for ncurses?
Cheers,
Nir.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 0/6] nconf bug fixes and U/E improvements
2011-08-31 7:46 ` [PATCH 0/6] nconf bug fixes and U/E improvements Cheng Renquan
2011-08-31 7:46 ` [PATCH V2 1/6] scripts/kconfig/nconf: fix typo: unknow => unknown Cheng Renquan
@ 2011-08-31 12:36 ` Cheng Renquan
1 sibling, 0 replies; 35+ messages in thread
From: Cheng Renquan @ 2011-08-31 12:36 UTC (permalink / raw)
To: linux-kbuild, Nir Tzachar, Arnaud Lacombe
Cc: Sam Ravnborg, Michal Marek, Randy Dunlap, c.rq541
On Wed, Aug 31, 2011 at 12:46 AM, Cheng Renquan <crquan@gmail.com> wrote:
> scripts/kconfig/nconf: add KEY_HOME / KEY_END for dialog_inputbox
> The nconfig even has KEY_UP as KEY_RIGHT and KEY_DOWN as KEY_LEFT,
> that I think more unnatural, better remove KEY_UP/KEY_DOWN in such
> a one line editor, or for future possible text area editor;
Hi Nir, how do you think the original design of UP as RIGHT and DOWN as LEFT?
in other applications, I saw DOWN as RIGHT and UP as LEFT; but generally
I think it's better to not support DOWN and UP;
Why is there need for KEY_UP/KEY_DOWN in a one line editor?
>
> scripts/kconfig/nconf: trunc too long string display in menu
>
> scripts/kconfig/nconf.c | 29 ++++++++++-------
> scripts/kconfig/nconf.gui.c | 73 ++++++++++++++++++++++++++++++++++++-------
> scripts/kconfig/nconf.h | 2 +-
> 3 files changed, 79 insertions(+), 25 deletions(-)
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH V2 6/6] scripts/kconfig/nconf: trunc too long string display in menu
2011-08-31 8:53 ` Nir Tzachar
@ 2011-08-31 13:04 ` Cheng Renquan
0 siblings, 0 replies; 35+ messages in thread
From: Cheng Renquan @ 2011-08-31 13:04 UTC (permalink / raw)
To: Nir Tzachar
Cc: linux-kbuild, Arnaud Lacombe, Sam Ravnborg, Michal Marek,
Randy Dunlap, c.rq541
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=UTF-8, Size: 3005 bytes --]
On Wed, Aug 31, 2011 at 1:53 AM, Nir Tzachar <nir.tzachar@gmail.com> wrote:
> Hi.
>
> On Wed, Aug 31, 2011 at 10:46 AM, Cheng Renquan <crquan@gmail.com> wrote:
>> When the string is too long, only show the first COLS/2 bytes wide,
>> to make sure there is some space to show the menu prompt.
>>
>> Signed-off-by: Cheng Renquan <crquan@gmail.com>
>> ---
>> Â scripts/kconfig/nconf.c | Â Â 8 +++++++-
>> Â 1 files changed, 7 insertions(+), 1 deletions(-)
>>
>> diff --git a/scripts/kconfig/nconf.c b/scripts/kconfig/nconf.c
>> index 4248759..4997ebf 100644
>> --- a/scripts/kconfig/nconf.c
>> +++ b/scripts/kconfig/nconf.c
>> @@ -887,8 +887,14 @@ static void build_conf(struct menu *menu)
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â break;
>> Â Â Â Â Â Â Â Â Â Â Â Â default:
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â tmp = 2 + strlen(sym_get_string_value(sym));
>> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â item_make(menu, 's', " Â Â (%s)",
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (tmp < COLS/2)
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â item_make(menu, 's', " Â Â (%s)",
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â sym_get_string_value(sym));
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â else {
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â char *s = strndupa(sym_get_string_value(sym), COLS/2);
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â strcpy(s + COLS/2 - 5, " ...");
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â item_make(menu, 's', " Â Â (%s)", s);
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â }
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â tmp = indent - tmp + 4;
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (tmp < 0)
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â tmp = 0;
>> --
>> 1.7.6
>>
>>
>
> Nack.
>
> I see several problems with this approach:
> 1) I do not like the use of strdupa.
Then strdup ? OK, strdupa may be glibc only but strdup should be generic,
> 2) I think it would be better to put this behavior in item_make(), as
> it will then be applied to all items, in all code paths. However, I a
> not sure that using COLS/2 is the best approach. Moreover, I think
> that this should be implemented inside the ncurses menu library and
> not worked around. Care to create a patch for ncurses?
In item_make it doesn't know if it will be followed by item_add_str,
but here our issue is sym value part too long, only in outside item_make
(the build_conf) it is aware about subsequent item_add_str, only there
we could reserve some space for menu prompt to pass to item_add_str;
but how much space do we better to reserve? the COLS/2 is just a
guessed value by me,
Even in patch for ncurses it can only truncate the string tail, no
help for our issue;
>
> Cheers,
> Nir.
ÿôèº{.nÇ+·®+%Ëÿ±éݶ\x17¥wÿº{.nÇ+·¥{±þFîWÿ{ayº\x1dÊÚë,j\a¢f£¢·hïêÿêçz_è®\x03(éÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?¨èÚ&£ø§~á
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH V3 1/5] scripts/kconfig/nconf: fix typo: unknow => unknown
2011-08-29 9:09 [RFC] nconf bug fixes and improvements Cheng Renquan
` (3 preceding siblings ...)
2011-08-31 7:46 ` [PATCH 0/6] nconf bug fixes and U/E improvements Cheng Renquan
@ 2011-09-01 17:52 ` crquan
2011-09-01 17:52 ` [PATCH V3 2/5] scripts/kconfig/nconf: fix memmove's length arg crquan
4 siblings, 1 reply; 35+ messages in thread
From: crquan @ 2011-09-01 17:52 UTC (permalink / raw)
To: linux-kbuild, Arnaud Lacombe
Cc: Sam Ravnborg, Michal Marek, Nir Tzachar, Randy Dunlap,
linux-kernel, c.rq541
From: Cheng Renquan <crquan@gmail.com>
Signed-off-by: Cheng Renquan <crquan@gmail.com>
Acked-by: Arnaud Lacombe <lacombar@gmail.com>
---
scripts/kconfig/nconf.gui.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/scripts/kconfig/nconf.gui.c b/scripts/kconfig/nconf.gui.c
index f8137b3..d3af04e 100644
--- a/scripts/kconfig/nconf.gui.c
+++ b/scripts/kconfig/nconf.gui.c
@@ -461,7 +461,7 @@ int dialog_inputbox(WINDOW *main_window,
result[cursor_position] = res;
cursor_position++;
} else {
- mvprintw(0, 0, "unknow key: %d\n", res);
+ mvprintw(0, 0, "unknown key: %d\n", res);
}
break;
}
--
1.7.6
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH V3 2/5] scripts/kconfig/nconf: fix memmove's length arg
2011-09-01 17:52 ` [PATCH V3 1/5] scripts/kconfig/nconf: fix typo: unknow => unknown crquan
@ 2011-09-01 17:52 ` crquan
2011-09-01 17:52 ` [PATCH V3 3/5] scripts/kconfig/nconf: dynamically alloc dialog_input_result crquan
0 siblings, 1 reply; 35+ messages in thread
From: crquan @ 2011-09-01 17:52 UTC (permalink / raw)
To: linux-kbuild, Arnaud Lacombe
Cc: Sam Ravnborg, Michal Marek, Nir Tzachar, Randy Dunlap,
linux-kernel, c.rq541
From: Cheng Renquan <crquan@gmail.com>
In case KEY_BACKSPACE / KEY_DC to delete a char, it memmove only
(len-cursor_position+1) bytes;
the default case is to insert a char, it should also memmove exactly
(len-cursor_position+1) bytes;
the original use of (len+1) is wrong and may access following memory
that doesn't belong to result, may cause SegFault in theory;
case KEY_BACKSPACE:
if (cursor_position > 0) {
memmove(&result[cursor_position-1],
&result[cursor_position],
len-cursor_position+1);
cursor_position--;
}
break;
case KEY_DC:
if (cursor_position >= 0 && cursor_position < len) {
memmove(&result[cursor_position],
&result[cursor_position+1],
len-cursor_position+1);
}
break;
default:
if ((isgraph(res) || isspace(res)) &&
len-2 < result_len) {
/* insert the char at the proper position */
memmove(&result[cursor_position+1],
&result[cursor_position],
len-cursor_position+1);
result[cursor_position] = res;
cursor_position++;
}
Signed-off-by: Cheng Renquan <crquan@gmail.com>
Acked-by: Nir Tzachar <nir.tzachar@gmail.com>
---
scripts/kconfig/nconf.gui.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/scripts/kconfig/nconf.gui.c b/scripts/kconfig/nconf.gui.c
index d3af04e..3ce2a7c 100644
--- a/scripts/kconfig/nconf.gui.c
+++ b/scripts/kconfig/nconf.gui.c
@@ -457,7 +457,7 @@ int dialog_inputbox(WINDOW *main_window,
/* insert the char at the proper position */
memmove(&result[cursor_position+1],
&result[cursor_position],
- len+1);
+ len-cursor_position+1);
result[cursor_position] = res;
cursor_position++;
} else {
--
1.7.6
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH V3 3/5] scripts/kconfig/nconf: dynamically alloc dialog_input_result
2011-09-01 17:52 ` [PATCH V3 2/5] scripts/kconfig/nconf: fix memmove's length arg crquan
@ 2011-09-01 17:52 ` crquan
2011-09-01 17:52 ` [PATCH V3 4/5] scripts/kconfig/nconf: fix editing long strings crquan
0 siblings, 1 reply; 35+ messages in thread
From: crquan @ 2011-09-01 17:52 UTC (permalink / raw)
To: linux-kbuild, Arnaud Lacombe
Cc: Sam Ravnborg, Michal Marek, Nir Tzachar, Randy Dunlap,
linux-kernel, c.rq541
From: Cheng Renquan <crquan@gmail.com>
To support unlimited length string config items;
No check for realloc return value keeps code simple, and to be
consistent with other existing unchecked malloc in kconfig.
Signed-off-by: Cheng Renquan <crquan@gmail.com>
Signed-off-by: Arnaud Lacombe <lacombar@gmail.com>
---
scripts/kconfig/nconf.c | 21 ++++++++++-----------
scripts/kconfig/nconf.gui.c | 20 +++++++++++++++-----
scripts/kconfig/nconf.h | 2 +-
3 files changed, 26 insertions(+), 17 deletions(-)
diff --git a/scripts/kconfig/nconf.c b/scripts/kconfig/nconf.c
index 39ca1f1..4248759 100644
--- a/scripts/kconfig/nconf.c
+++ b/scripts/kconfig/nconf.c
@@ -280,6 +280,9 @@ static int global_exit;
/* the currently selected button */
const char *current_instructions = menu_instructions;
+static char *dialog_input_result;
+static int dialog_input_result_len;
+
static void conf(struct menu *menu);
static void conf_choice(struct menu *menu);
static void conf_string(struct menu *menu);
@@ -695,7 +698,6 @@ static void search_conf(void)
{
struct symbol **sym_arr;
struct gstr res;
- char dialog_input_result[100];
char *dialog_input;
int dres;
again:
@@ -703,7 +705,7 @@ again:
_("Search Configuration Parameter"),
_("Enter " CONFIG_ " (sub)string to search for "
"(with or without \"" CONFIG_ "\")"),
- "", dialog_input_result, 99);
+ "", &dialog_input_result, &dialog_input_result_len);
switch (dres) {
case 0:
break;
@@ -1348,7 +1350,6 @@ static void conf_choice(struct menu *menu)
static void conf_string(struct menu *menu)
{
const char *prompt = menu_get_prompt(menu);
- char dialog_input_result[256];
while (1) {
int res;
@@ -1371,8 +1372,8 @@ static void conf_string(struct menu *menu)
prompt ? _(prompt) : _("Main Menu"),
heading,
sym_get_string_value(menu->sym),
- dialog_input_result,
- sizeof(dialog_input_result));
+ &dialog_input_result,
+ &dialog_input_result_len);
switch (res) {
case 0:
if (sym_set_string_value(menu->sym,
@@ -1392,14 +1393,13 @@ static void conf_string(struct menu *menu)
static void conf_load(void)
{
- char dialog_input_result[256];
while (1) {
int res;
res = dialog_inputbox(main_window,
NULL, load_config_text,
filename,
- dialog_input_result,
- sizeof(dialog_input_result));
+ &dialog_input_result,
+ &dialog_input_result_len);
switch (res) {
case 0:
if (!dialog_input_result[0])
@@ -1424,14 +1424,13 @@ static void conf_load(void)
static void conf_save(void)
{
- char dialog_input_result[256];
while (1) {
int res;
res = dialog_inputbox(main_window,
NULL, save_config_text,
filename,
- dialog_input_result,
- sizeof(dialog_input_result));
+ &dialog_input_result,
+ &dialog_input_result_len);
switch (res) {
case 0:
if (!dialog_input_result[0])
diff --git a/scripts/kconfig/nconf.gui.c b/scripts/kconfig/nconf.gui.c
index 3ce2a7c..d64bc1c 100644
--- a/scripts/kconfig/nconf.gui.c
+++ b/scripts/kconfig/nconf.gui.c
@@ -356,7 +356,7 @@ int btn_dialog(WINDOW *main_window, const char *msg, int btn_num, ...)
int dialog_inputbox(WINDOW *main_window,
const char *title, const char *prompt,
- const char *init, char *result, int result_len)
+ const char *init, char **resultp, int *result_len)
{
int prompt_lines = 0;
int prompt_width = 0;
@@ -367,7 +367,12 @@ int dialog_inputbox(WINDOW *main_window,
int i, x, y;
int res = -1;
int cursor_position = strlen(init);
+ char *result = *resultp;
+ if (strlen(init)+1 > *result_len) {
+ *result_len = strlen(init)+1;
+ *resultp = result = realloc(result, *result_len);
+ }
/* find the widest line of msg: */
prompt_lines = get_line_no(prompt);
@@ -384,7 +389,7 @@ int dialog_inputbox(WINDOW *main_window,
y = (LINES-(prompt_lines+4))/2;
x = (COLS-(prompt_width+4))/2;
- strncpy(result, init, result_len);
+ strncpy(result, init, *result_len);
/* create the windows */
win = newwin(prompt_lines+6, prompt_width+7, y, x);
@@ -443,7 +448,7 @@ int dialog_inputbox(WINDOW *main_window,
case KEY_UP:
case KEY_RIGHT:
if (cursor_position < len &&
- cursor_position < min(result_len, prompt_width))
+ cursor_position < min(*result_len, prompt_width))
cursor_position++;
break;
case KEY_DOWN:
@@ -452,8 +457,13 @@ int dialog_inputbox(WINDOW *main_window,
cursor_position--;
break;
default:
- if ((isgraph(res) || isspace(res)) &&
- len-2 < result_len) {
+ if ((isgraph(res) || isspace(res))) {
+ /* one for new char, one for '\0' */
+ if (len+2 > *result_len) {
+ *result_len = len+2;
+ *resultp = result = realloc(result,
+ *result_len);
+ }
/* insert the char at the proper position */
memmove(&result[cursor_position+1],
&result[cursor_position],
diff --git a/scripts/kconfig/nconf.h b/scripts/kconfig/nconf.h
index 58fbda8..0d52617 100644
--- a/scripts/kconfig/nconf.h
+++ b/scripts/kconfig/nconf.h
@@ -89,7 +89,7 @@ void fill_window(WINDOW *win, const char *text);
int btn_dialog(WINDOW *main_window, const char *msg, int btn_num, ...);
int dialog_inputbox(WINDOW *main_window,
const char *title, const char *prompt,
- const char *init, char *result, int result_len);
+ const char *init, char **resultp, int *result_len);
void refresh_all_windows(WINDOW *main_window);
void show_scroll_win(WINDOW *main_window,
const char *title,
--
1.7.6
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH V3 4/5] scripts/kconfig/nconf: fix editing long strings
2011-09-01 17:52 ` [PATCH V3 3/5] scripts/kconfig/nconf: dynamically alloc dialog_input_result crquan
@ 2011-09-01 17:52 ` crquan
2011-09-01 17:52 ` [PATCH V3 5/5] scripts/kconfig/nconf: add KEY_HOME / KEY_END for dialog_inputbox crquan
0 siblings, 1 reply; 35+ messages in thread
From: crquan @ 2011-09-01 17:52 UTC (permalink / raw)
To: linux-kbuild, Arnaud Lacombe
Cc: Sam Ravnborg, Michal Marek, Nir Tzachar, Randy Dunlap,
linux-kernel, c.rq541
From: Cheng Renquan <crquan@gmail.com>
The original dialog_inputbox doesn't work with longer than prompt_width
strings, here fixed it in this way:
1) add variable cursor_form_win to record cursor of form_win,
keep its value always between [0, prompt_width-1];
reuse the original cursor_position as cursor of the string result,
use (cursor_position-cursor_form_win) as begin offset to show part of
the string in form_win;
Signed-off-by: Cheng Renquan <crquan@gmail.com>
Cc: Arnaud Lacombe <lacombar@gmail.com>
Cc: Nir Tzachar <nir.tzachar@gmail.com>
---
scripts/kconfig/nconf.gui.c | 29 +++++++++++++++++++++++------
1 files changed, 23 insertions(+), 6 deletions(-)
diff --git a/scripts/kconfig/nconf.gui.c b/scripts/kconfig/nconf.gui.c
index d64bc1c..4b9d8b6 100644
--- a/scripts/kconfig/nconf.gui.c
+++ b/scripts/kconfig/nconf.gui.c
@@ -367,6 +367,7 @@ int dialog_inputbox(WINDOW *main_window,
int i, x, y;
int res = -1;
int cursor_position = strlen(init);
+ int cursor_form_win;
char *result = *resultp;
if (strlen(init)+1 > *result_len) {
@@ -410,7 +411,9 @@ int dialog_inputbox(WINDOW *main_window,
fill_window(prompt_win, prompt);
mvwprintw(form_win, 0, 0, "%*s", prompt_width, " ");
- mvwprintw(form_win, 0, 0, "%s", result);
+ cursor_form_win = min(cursor_position, prompt_width-1);
+ mvwprintw(form_win, 0, 0, "%s",
+ result + cursor_position-cursor_form_win);
/* create panels */
panel = new_panel(win);
@@ -436,6 +439,8 @@ int dialog_inputbox(WINDOW *main_window,
&result[cursor_position],
len-cursor_position+1);
cursor_position--;
+ cursor_form_win--;
+ len--;
}
break;
case KEY_DC:
@@ -443,18 +448,22 @@ int dialog_inputbox(WINDOW *main_window,
memmove(&result[cursor_position],
&result[cursor_position+1],
len-cursor_position+1);
+ len--;
}
break;
case KEY_UP:
case KEY_RIGHT:
- if (cursor_position < len &&
- cursor_position < min(*result_len, prompt_width))
+ if (cursor_position < len) {
cursor_position++;
+ cursor_form_win++;
+ }
break;
case KEY_DOWN:
case KEY_LEFT:
- if (cursor_position > 0)
+ if (cursor_position > 0) {
cursor_position--;
+ cursor_form_win--;
+ }
break;
default:
if ((isgraph(res) || isspace(res))) {
@@ -470,16 +479,24 @@ int dialog_inputbox(WINDOW *main_window,
len-cursor_position+1);
result[cursor_position] = res;
cursor_position++;
+ cursor_form_win++;
+ len++;
} else {
mvprintw(0, 0, "unknown key: %d\n", res);
}
break;
}
+ if (cursor_form_win < 0)
+ cursor_form_win = 0;
+ else if (cursor_form_win > prompt_width-1)
+ cursor_form_win = prompt_width-1;
+
wmove(form_win, 0, 0);
wclrtoeol(form_win);
mvwprintw(form_win, 0, 0, "%*s", prompt_width, " ");
- mvwprintw(form_win, 0, 0, "%s", result);
- wmove(form_win, 0, cursor_position);
+ mvwprintw(form_win, 0, 0, "%s",
+ result + cursor_position-cursor_form_win);
+ wmove(form_win, 0, cursor_form_win);
touchwin(win);
refresh_all_windows(main_window);
--
1.7.6
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH V3 5/5] scripts/kconfig/nconf: add KEY_HOME / KEY_END for dialog_inputbox
2011-09-01 17:52 ` [PATCH V3 4/5] scripts/kconfig/nconf: fix editing long strings crquan
@ 2011-09-01 17:52 ` crquan
2011-09-09 12:46 ` Michal Marek
0 siblings, 1 reply; 35+ messages in thread
From: crquan @ 2011-09-01 17:52 UTC (permalink / raw)
To: linux-kbuild, Arnaud Lacombe
Cc: Sam Ravnborg, Michal Marek, Nir Tzachar, Randy Dunlap,
linux-kernel, c.rq541
From: Cheng Renquan <crquan@gmail.com>
to make it easier to locate begin/end when editing long strings;
Signed-off-by: Cheng Renquan <crquan@gmail.com>
Acked By: Nir Tzachar <nir.tzachar@gmail.com>
---
scripts/kconfig/nconf.gui.c | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/scripts/kconfig/nconf.gui.c b/scripts/kconfig/nconf.gui.c
index 4b9d8b6..3b18dd8 100644
--- a/scripts/kconfig/nconf.gui.c
+++ b/scripts/kconfig/nconf.gui.c
@@ -465,6 +465,14 @@ int dialog_inputbox(WINDOW *main_window,
cursor_form_win--;
}
break;
+ case KEY_HOME:
+ cursor_position = 0;
+ cursor_form_win = 0;
+ break;
+ case KEY_END:
+ cursor_position = len;
+ cursor_form_win = min(cursor_position, prompt_width-1);
+ break;
default:
if ((isgraph(res) || isspace(res))) {
/* one for new char, one for '\0' */
--
1.7.6
^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [PATCH V3 5/5] scripts/kconfig/nconf: add KEY_HOME / KEY_END for dialog_inputbox
2011-09-01 17:52 ` [PATCH V3 5/5] scripts/kconfig/nconf: add KEY_HOME / KEY_END for dialog_inputbox crquan
@ 2011-09-09 12:46 ` Michal Marek
0 siblings, 0 replies; 35+ messages in thread
From: Michal Marek @ 2011-09-09 12:46 UTC (permalink / raw)
To: crquan
Cc: linux-kbuild, Arnaud Lacombe, Sam Ravnborg, Nir Tzachar,
Randy Dunlap, linux-kernel, c.rq541
On 1.9.2011 19:52, crquan@gmail.com wrote:
> From: Cheng Renquan <crquan@gmail.com>
>
> to make it easier to locate begin/end when editing long strings;
>
> Signed-off-by: Cheng Renquan <crquan@gmail.com>
> Acked By: Nir Tzachar <nir.tzachar@gmail.com>
> --
Applied all five to kbuild-2.6.git#kconfig.
Michal
^ permalink raw reply [flat|nested] 35+ messages in thread
end of thread, other threads:[~2011-09-09 12:46 UTC | newest]
Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-29 9:09 [RFC] nconf bug fixes and improvements Cheng Renquan
2011-08-29 14:14 ` Arnaud Lacombe
2011-08-29 14:14 ` Arnaud Lacombe
2011-08-29 16:12 ` Randy Dunlap
2011-08-29 16:53 ` Sam Ravnborg
2011-08-29 23:56 ` [PATCH 1/5] scripts/kconfig/nconf: fix typo: unknow => unknown Cheng Renquan
2011-08-29 23:56 ` [PATCH 2/5] scripts/kconfig/nconf: fix memmove's length arg Cheng Renquan
2011-08-29 23:56 ` [PATCH 3/5] scripts/kconfig/nconf: dynamically alloc dialog_input_result Cheng Renquan
2011-08-29 23:56 ` [PATCH 4/5] scripts/kconfig/nconf: fix editing long strings Cheng Renquan
2011-08-29 23:56 ` [PATCH 5/5] scripts/kconfig/nconf: add KEY_HOME / KEY_END for dialog_inputbox Cheng Renquan
2011-08-30 4:59 ` [PATCH 4/5] scripts/kconfig/nconf: fix editing long strings Arnaud Lacombe
2011-08-31 8:39 ` Nir Tzachar
2011-08-30 0:16 ` [PATCH 3/5] scripts/kconfig/nconf: dynamically alloc dialog_input_result Cheng Renquan
2011-08-30 3:25 ` Arnaud Lacombe
2011-08-30 4:14 ` Arnaud Lacombe
2011-08-30 4:26 ` Arnaud Lacombe
2011-08-30 0:02 ` [PATCH 1/5] scripts/kconfig/nconf: fix typo: unknow => unknown Arnaud Lacombe
2011-08-31 7:46 ` [PATCH 0/6] nconf bug fixes and U/E improvements Cheng Renquan
2011-08-31 7:46 ` [PATCH V2 1/6] scripts/kconfig/nconf: fix typo: unknow => unknown Cheng Renquan
2011-08-31 7:46 ` [PATCH V2 2/6] scripts/kconfig/nconf: fix memmove's length arg Cheng Renquan
2011-08-31 7:46 ` [PATCH V2 3/6] scripts/kconfig/nconf: dynamically alloc dialog_input_result Cheng Renquan
2011-08-31 7:46 ` [PATCH V2 4/6] scripts/kconfig/nconf: fix editing long strings Cheng Renquan
2011-08-31 7:46 ` [PATCH V2 5/6] scripts/kconfig/nconf: add KEY_HOME / KEY_END for dialog_inputbox Cheng Renquan
2011-08-31 7:46 ` [PATCH V2 6/6] scripts/kconfig/nconf: trunc too long string display in menu Cheng Renquan
2011-08-31 8:53 ` Nir Tzachar
2011-08-31 13:04 ` Cheng Renquan
2011-08-31 8:42 ` [PATCH V2 5/6] scripts/kconfig/nconf: add KEY_HOME / KEY_END for dialog_inputbox Nir Tzachar
2011-08-31 8:30 ` [PATCH V2 2/6] scripts/kconfig/nconf: fix memmove's length arg Nir Tzachar
2011-08-31 12:36 ` [PATCH 0/6] nconf bug fixes and U/E improvements Cheng Renquan
2011-09-01 17:52 ` [PATCH V3 1/5] scripts/kconfig/nconf: fix typo: unknow => unknown crquan
2011-09-01 17:52 ` [PATCH V3 2/5] scripts/kconfig/nconf: fix memmove's length arg crquan
2011-09-01 17:52 ` [PATCH V3 3/5] scripts/kconfig/nconf: dynamically alloc dialog_input_result crquan
2011-09-01 17:52 ` [PATCH V3 4/5] scripts/kconfig/nconf: fix editing long strings crquan
2011-09-01 17:52 ` [PATCH V3 5/5] scripts/kconfig/nconf: add KEY_HOME / KEY_END for dialog_inputbox crquan
2011-09-09 12:46 ` Michal Marek
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.