[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] allow user-configurable menucolor
From: |
Vesa Jääskeläinen |
Subject: |
Re: [PATCH] allow user-configurable menucolor |
Date: |
Tue, 01 Jan 2008 15:51:39 +0200 |
User-agent: |
Thunderbird 2.0.0.9 (Windows/20071031) |
Robert Millan wrote:
> On Tue, Jan 01, 2008 at 03:14:50PM +0200, Vesa Jääskeläinen wrote:
>> First some questions ;)
>>
>> - What happens when you forgot to provide / in this string ;) ?
>
> grub_strchr() returns NULL and... whoops! I'll fix that :-)
>
>> - What happens when all memory is used?
>
> What do you mean?
strdup() == NULL
>> - What happens when you alter list of colors... remove one entry of it
>> or add one.
>
> 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 neither
> should this list.
Yes, but... (below)
>> (eg. hard coding is bad... sizeof...)
>
> Where can sizeof be used?
for (i = 0; i < 0x10; i++)
->
for (i = 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 following:
>>
>> parse_single_color_name -> parse_color_name
>>
>> parse_color_name -> parse_color_tuple, parse_color_pair,
>> parse_text_color or parse_menu_color
>
> How about `parse_color_name_pair' ?
this name is good as any and goes well with parse_color_name name.
- [PATCH] allow user-configurable menucolor, Robert Millan, 2008/01/01
- Re: [PATCH] allow user-configurable menucolor, Vesa Jääskeläinen, 2008/01/01
- Re: [PATCH] allow user-configurable menucolor, Robert Millan, 2008/01/01
- Re: [PATCH] allow user-configurable menucolor,
Vesa Jääskeläinen <=
- Re: [PATCH] allow user-configurable menucolor, Robert Millan, 2008/01/01
- Re: [PATCH] allow user-configurable menucolor, Robert Millan, 2008/01/01
- Re: [PATCH] allow user-configurable menucolor, Robert Millan, 2008/01/02
- Re: [PATCH] allow user-configurable menucolor, Yoshinori K. Okuji, 2008/01/02
- Re: [PATCH] allow user-configurable menucolor, Robert Millan, 2008/01/02
Re: [PATCH] allow user-configurable menucolor, Yoshinori K. Okuji, 2008/01/02