[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20110829091218.84ac35f5.rdunlap@xenotime.net>
Date: Mon, 29 Aug 2011 09:12:18 -0700
From: Randy Dunlap <rdunlap@...otime.net>
To: Arnaud Lacombe <lacombar@...il.com>
Cc: Cheng Renquan <crquan@...il.com>, Michal Marek <mmarek@...e.cz>,
linux-kbuild@...r.kernel.org, linux-kernel@...r.kernel.org,
Nir Tzachar <nir.tzachar@...il.com>
Subject: Re: [RFC] nconf bug fixes and improvements
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@...il.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 ***
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists