From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with archive (Exim 4.43) id 1E4mDv-0002Vv-Ti for mharc-grub-devel@gnu.org; Mon, 15 Aug 2005 17:10:23 -0400 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1E4mDn-0002NN-WD for grub-devel@gnu.org; Mon, 15 Aug 2005 17:10:16 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1E4mDi-0002Jj-Nu for grub-devel@gnu.org; Mon, 15 Aug 2005 17:10:13 -0400 Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1E4mDi-0002JG-Kn for grub-devel@gnu.org; Mon, 15 Aug 2005 17:10:10 -0400 Received: from [194.251.242.244] (helo=fep19.inet.fi) by monty-python.gnu.org with esmtp (Exim 4.34) id 1E4mOg-0007jZ-Vs for grub-devel@gnu.org; Mon, 15 Aug 2005 17:21:31 -0400 Received: from [80.222.166.89] by fep19.inet.fi with ESMTP id <20050815210538.XKEE9928.fep19.inet.fi@[80.222.166.89]> for ; Tue, 16 Aug 2005 00:05:38 +0300 Message-ID: <430103AD.1090800@nic.fi> Date: Tue, 16 Aug 2005 00:05:49 +0300 From: =?ISO-8859-1?Q?Vesa_J=E4=E4skel=E4inen?= User-Agent: Thunderbird 1.0+ (Windows/20050809) MIME-Version: 1.0 To: The development of GRUB 2 References: <42FF3097.7030904@nic.fi> <87zmrjxge4.fsf@student.han.nl> In-Reply-To: <87zmrjxge4.fsf@student.han.nl> X-Enigmail-Version: 0.92.0.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: vesafb terminal for testing. 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: Mon, 15 Aug 2005 21:10:22 -0000 Marco Gerards wrote: > Some comments on the patch: > >> 2005-08-14 Vesa Jaaskelainen >> >> + * 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 >> + >> * 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.