Re: bugfix, hostfs

[Marco Gerards]

address@hidden (Tomas Ebenlendr) writes:

> I don't *need* it, it is just comfortable way to access files from
> grub-emu. I also don't *need* to load modules in grub-emu, but if it
> will be easy and not user-confusing, it will make easier solving some
> problems.

Ok, I just asked out of interest.

>> Most comments are still about the GCS.
>
> Hmm, I wrote the patch for myself, and then I forgot to read it
> carefully. So I'm sory for inconvenient code.

np.  I am happy you took the time to do this work. :)

>> > +#ifndef GRUB_UTIL
>> > +#error cannot live outside host fs
>> > +#endif
>> 
>> I think there is no need to do this.
>> 
>
> Maybe. I'm just used to write in files that shouldn't be ported to other
> "parts" of software that they can't be ported.

If someone would compile this code for usage outside of grub-emu it
won't compile anyway. :)

>> > +  if ((signed) device->disk->id != -2) {
>> 
>> What is -2?
>>
>
> Oh, sorry. Just a magic constant. Probably there should be better
> identification (e.g. magic device->disk->data (e.g. device->disk ==
> device->disk->data)). This identification is used in hostfs_mount()

Ok.  If you can't do it that way it is better to use a macro like
GRUB_HOSTFS_DISK_ID or so.

>> 
>> > +  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).
>> 
>
> I concatenate path of directory and its entries to stat them. I will use
> the MAX_PATH_LEN. I only forgot that I forgot the name of this constant.
> (Uh).

Oh, I thought it wasn't written to.  I must have missed something
then.  Please be careful with MAX_PATH_LEN, GNU/Hurd does not define
it because there is no limit at all.  In that case better use dynamic
allocation.

Thanks,
Marco