Re: hostfs patch

[Marco Gerards]

Tomas Ebenlendr <address@hidden> writes:

Hi Tomas,

Here are a few more comments.  Most are related to the GCS and there
are some typos in the new comments.

>       * fs/hostfs.c: New file. Connects grub fs interface and libc file
>       interface.
>       * util/fakedisk.c: New file, implements fake disks.

I think we discussed the location before.  I am not sure, but the
files that use POSIX calls should be in util/, the others should be in
either fs/ or disks/.  I see something specific to libc in fs/.
Perhaps I just told you the wrong thing last time because I
misunderstood things...

>       * include/grub/fs.h (grub_hostfs_init, grub_hostfs_fini): New
>       prototypes.
>       * include/grub/fakedisk.h: New file.
>       * include/grub/disk.h: New constant GRUB_DISK_DEVICE_FAKEDISK_ID.

This should be:

        * include/grub/disk.h (enum grub_disk_dev_id): New member
          `GRUB_DISK_DEVICE_FAKEDISK_ID'.

>       * util/grub-emu.c (main): Added inicialization and destruction

Typo.

> +  /* This shoud be much greater than zero. Some fs claims that disk is
> +     totaly bad while fs-probe if total_sectors to low.  */
> +  disk->total_sectors = 1234;

Typo: sould -> should

Please change this to 0 or so.  In that case filesystems can't read
from it in any case.  And the filesystems that do that should be
fixed, I plan to do that soon.  I've encountered this same problem
elsewhere.

> +  grub_fakedisk_entry_t d = fakedisks, *p = &fakedisks;

Please split this up.

> +grub_err_t grub_fakedisk_register (const char * name);
> +/* Remove one fakedisk device.  */
> +void grub_fakedisk_unregister (const char * name);

Here you have a space between the `*' and the variable (name in this
case).  Please do not do this.  Can you have a look if you do the same
in other places in the code?   I am sure i missed some.

Thanks,
Marco