From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with archive (Exim 4.33) id 1Bts2B-0005Kj-VC for mharc-grub-devel@gnu.org; Sun, 08 Aug 2004 14:04:40 -0400 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.33) id 1Bts29-0005Hn-V9 for grub-devel@gnu.org; Sun, 08 Aug 2004 14:04:38 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.33) id 1Bts27-0005Fs-W7 for grub-devel@gnu.org; Sun, 08 Aug 2004 14:04:37 -0400 Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.33) id 1Bts27-0005FZ-TS for grub-devel@gnu.org; Sun, 08 Aug 2004 14:04:35 -0400 Received: from [145.74.66.11] (helo=mail-cn.han.nl) by monty-python.gnu.org with esmtp (Exim 4.34) id 1BtryJ-0006EV-AI for grub-devel@gnu.org; Sun, 08 Aug 2004 14:00:39 -0400 Received: from localhost (charlie.han.nl [145.74.66.9]) by mail-cn.han.nl (Postfix) with ESMTP id 2EFD1810D for ; Sun, 8 Aug 2004 20:00:33 +0200 (CEST) Received: from mail-cn.han.nl ([145.74.66.11]) by localhost (charlie.han.nl [145.74.66.9]) (amavisd-new, port 10024) with ESMTP id 20044-01 for ; Sun, 8 Aug 2004 20:00:29 +0200 (CEST) Received: from mail1.han.nl (mail1.han.nl [145.74.103.11]) by mail-cn.han.nl (Postfix) with ESMTP id DF7488132 for ; Sun, 8 Aug 2004 20:00:29 +0200 (CEST) Received: from marco.marco-g.com (a82-92-27-129.adsl.xs4all.nl [82.92.27.129]) by mail1.han.nl (Postfix) with ESMTP id 67932C045 for ; Sun, 8 Aug 2004 19:00:32 +0200 (CEST) Mail-Copies-To: metgerards@student.han.nl To: grub-devel@gnu.org References: <20040808173040.GA11531@artax.karlin.mff.cuni.cz> From: Marco Gerards Date: Sun, 08 Aug 2004 20:00:53 +0200 In-Reply-To: <20040808173040.GA11531@artax.karlin.mff.cuni.cz> (Tomas Ebenlendr's message of "Sun, 8 Aug 2004 19:30:40 +0200") Message-ID: <87smaxldbe.fsf@marco.marco-g.com> User-Agent: Gnus/5.1006 (Gnus v5.10.6) Emacs/21.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Virus-Scanned: by amavisd-new@vscan-cn.han.nl Subject: Re: bugfix, hostfs 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: Sun, 08 Aug 2004 18:04:38 -0000 ebik@artax.karlin.mff.cuni.cz (Tomas Ebenlendr) writes: > I'm thinkig about module loading in grub-emu (now I'm exploring dl.c, ld > and so on) and as side efect I wrote access to host filesystem for grub. > This also discovered a bug in dl.c which occur when 'normal.mod' exists, > but cannot be loaded. Nice! Thanks for your patches. Why do you need hostfs for this? I think hostfs can be really useful to me for debugging etc., but is it required for module loading? Anyway, it would be really useful for me when I am writing filesystem implementations. Here is a quick review of your patches. As I do not have much time now I hope someone else can provide better comments. Most comments are still about the GCS. If I will commit the patch (I can do so if Okuji agrees with the patches), I will fix the GCS related problems as well. Hopefully you will read the GCS comments to see our coding style. > mod = grub_dl_load_core (core, size); > - mod->ref_count = 0; > + if (mod) mod->ref_count = 0; AFAIK you should split this up according to the GCS. Like this: if (mod) mod->ref_count = 0; > file conf/i386-pc.mk should be generated before commiting, > file conf/powerpc-ieee1275.rmk should be patched same way as i386-pc.rmk Ok. > +#ifndef GRUB_UTIL > +#error cannot live outside host fs > +#endif I think there is no need to do this. > + if ((signed) device->disk->id != -2) { What is -2? > +static grub_err_t > +grub_host_dir (grub_device_t device, const char *path, > + int (*hook) (const char *filename, int dir)) > +{ > + DIR * dir; > + struct dirent * dent; Use: DIR *dir; (no space) > + struct stat stent; > + static char pathbuf[/*FIXME*/2048 + NAME_MAX]; > + char * ename=pathbuf; > + > + if (host_mount(device)) return grub_errno; A space between the function name and the "(". > + grub_strncpy(pathbuf,path,/*FIXME*/2048 - 1); Why do you use pathbuf? Can't you just use path directly? If that is not possible use MAX_PATH_LEN here when it is defined and dynamic memory allocation otherwise (when there is no limit). > + while (*ename) ename++;/* let ename point after 'path' */ Please write this like this: /* Let ename point after PATH. */ while (*ename) ename++; > + ename[NAME_MAX]=0; Please add a space before and after the '='. Thanks, Marco