Re: vesafb terminal for testing.

["Vesa Jääskeläinen" <chaac@nic.fi>]

Marco Gerards wrote:
> Some comments on the patch:
> 
>>  2005-08-14  Vesa Jaaskelainen  <chaac@nic.fi>
>>  
>> +	* DISTLIST: Added term/i386/pc/vesafb.c
>> +	* term/i386/pc/vesafb: New file.
> 
> Can you please split this up to the generic framebuffer terminal?  The
> terminal for framebuffers should be generic and the videodrivers
> should be arch specific.  Or do you want to do that later?

I'll do that later. Let's first test this beast out and when it works
then design the actual video driver. I have tried to make framebuffer
code as separate as I could have in this point.

> You forgot the ".c" on the last line.

Already fixed. Actually I noticed that typo when I first sent the patch.

>> +
>> +2005-08-14  Vesa Jaaskelainen  <chaac@nic.fi>
>> +
>>  	* DISTLIST: Added include/grub/i386/pc/vbe.h.
> 
> Why two changelog entries?  I do not see this change anywhere in the
> patch.

Before making that patch I committed some fixes to DISTLIST and that was
last entry in Changelog before starting to migrate my patch to newest
GRUB. Date on those two patches were identical so diff got "confused"
about those.

>> +static void
>> +*real2pm(grub_vbe_farptr_t ptr)
> 
> The * is a part of the type, can you put it on the previous line?

Sure. It seems that I already spotted that one :)

> There are some long lines, can you please make them shorter so
> the maximal line length is 78 characters?

Hmm... I made another scan for those. Still I left some of those there
because in my opinion it is much more readable in that way. But splitted
some lines.

> You sometimes use C++ style comments.  Can you change your comments so
> they match the style described in the GCS:
> http://www.gnu.org/prep/standards/html_node/Comments.html#Comments

I think there should not be anymore C++ style comments. It's hard to
stay away from ooooold habits :)

Fortunely I found GNU indentation mode from my editor :)... It makes
this a bit easier.

I will scan the code once more and then commit a bit different version
to CVS as Okuji asked to commit it to there for easier testing.