Re: [RFC] nconf bug fixes and improvements

[Arnaud Lacombe]

Hi,

On Mon, Aug 29, 2011 at 5:09 AM, Cheng Renquan <crquan@xxxxxxxxx> 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@xxxxxxxxxxxxxxx
> More majordomo info at Âhttp://vger.kernel.org/majordomo-info.html
>
N‹§²æìr¸›yúèšØb²X¬¶ÇvØ^–)Þ{.nÇ+‰·¥Š{±‘êçzX§¶›¡Ü}©ž²ÆzÚ&j:+v‰¨¾«‘êçzZ+€Ê+zf£¢·hšˆ§~†­†Ûiÿûàz¹®w¥¢¸?™¨è­Ú&¢)ßf”ù^jÇy§m…á@A«a¶Úÿ0¶ìh®å’i