From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with archive (Exim 4.43) id 1J9hWq-0007ID-7I for mharc-grub-devel@gnu.org; Tue, 01 Jan 2008 08:51:36 -0500 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1J9hWo-0007GF-5U for grub-devel@gnu.org; Tue, 01 Jan 2008 08:51:34 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1J9hWn-0007F7-CO for grub-devel@gnu.org; Tue, 01 Jan 2008 08:51:33 -0500 Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1J9hWn-0007Es-3b for grub-devel@gnu.org; Tue, 01 Jan 2008 08:51:33 -0500 Received: from pne-smtpout3-sn2.hy.skanova.net ([81.228.8.111]) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1J9hWm-0007Nx-M8 for grub-devel@gnu.org; Tue, 01 Jan 2008 08:51:32 -0500 Received: from [127.0.0.1] (88.193.32.97) by pne-smtpout3-sn2.hy.skanova.net (7.3.129) id 471A5695003B8868 for grub-devel@gnu.org; Tue, 1 Jan 2008 14:50:50 +0100 Message-ID: <477A456B.2010209@nic.fi> Date: Tue, 01 Jan 2008 15:51:39 +0200 From: =?UTF-8?B?VmVzYSBKw6TDpHNrZWzDpGluZW4=?= User-Agent: Thunderbird 2.0.0.9 (Windows/20071031) MIME-Version: 1.0 To: The development of GRUB 2 References: <20080101124728.GA30417@thorin> <477A3CCA.4090908@nic.fi> <20080101134026.GA32057@thorin> In-Reply-To: <20080101134026.GA32057@thorin> X-Enigmail-Version: 0.95.5 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: Quoted-Printable X-detected-kernel: by monty-python.gnu.org: Solaris 10 (beta) Subject: Re: [PATCH] allow user-configurable menucolor X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: The development of GRUB 2 List-Id: The development of GRUB 2 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 01 Jan 2008 13:51:34 -0000 Robert Millan wrote: > On Tue, Jan 01, 2008 at 03:14:50PM +0200, Vesa J=C3=A4=C3=A4skel=C3=A4i= nen wrote: >> First some questions ;) >> >> - What happens when you forgot to provide / in this string ;) ? >=20 > grub_strchr() returns NULL and... whoops! I'll fix that :-) >=20 >> - What happens when all memory is used? >=20 > What do you mean? strdup() =3D=3D NULL >> - What happens when you alter list of colors... remove one entry of it >> or add one. >=20 > This should never happen. The list is just a human description of GRUB= color > codes. GRUB VGA-style color codes are never changed in runtime, so nei= ther > should this list. Yes, but... (below) >> (eg. hard coding is bad... sizeof...) >=20 > Where can sizeof be used? for (i =3D 0; i < 0x10; i++) -> for (i =3D 0; i < sizeof(color_list) / sizeof(*color_list); i++) or similar compile time calculation for it. If you are using construct like this then your code always indexes array correctly. This of course requires that color_list is local to source file being compiled. Point being taken here is that never use hard-coded indexes to some arrays when there is even slight possibility that it can change. >> Now to your naming issue... >> >> This code is quite specific to be only local so I would propose follow= ing: >> >> parse_single_color_name -> parse_color_name >> >> parse_color_name -> parse_color_tuple, parse_color_pair, >> parse_text_color or parse_menu_color >=20 > How about `parse_color_name_pair' ? this name is good as any and goes well with parse_color_name name.