grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: NTFS file system driver (update 3)


From: Marco Gerards
Subject: Re: NTFS file system driver (update 3)
Date: Sun, 29 Jul 2007 22:31:57 +0200
User-agent: Gnus/5.110006 (No Gnus v0.6) Emacs/21.4 (gnu/linux)

Bean <address@hidden> writes:

> On Sun, Jul 22, 2007 at 02:18:23PM +0200, Marco Gerards wrote:
>> Was this patch tested on both 32 and 64 bits machines?  And little/big
>> endian machines?  Perhaps other people can help Bean if he doesn't
>> have access to such machines?
>
> I only test it in 32-bit x86 machines, please report bug if it doesn't work
> in other environment.

Did someone test this?

>> One comment in general is that you do not use fshelp.h.  Could you
>> please have a quick look at this?  This would help to structure the
>> code a bit more like the other filesystem implementations and save
>> some code and complexity.  If it can be used (in a sane way), please
>> do :-)
>
> I've rewritten the directory searching code to use grub_fshelp_find_file. But
> I don't use grub_fshelp_read_file to read files, becasue the fshelp
> implementation don't work well with ntfs filesystem.
>
>> What are you caching and how would that work?
>
> I'm caching the last read cluster, which can be 512 to 4192 long. If the next
> read is from the same cluster, no disk access is needed.

GRUB already has caching code, see kern/disk.c.  So this just can
better be removed, to fix deallocation issues, etc.

> The buffer is attr->sbuf, it's released in free_attr.
>
>> Why are you using these hooks?  I think they are meant for the users
>> of the filesystem, so they can build a blocklist or so and not for the
>> filesystems.  Are you really sure this is required?
>
> The hook is used to get the disk layout of a file. But sometimes the driver
> needs to read from system structure like mft. So I'm enabling the hook only
> when it's reading data blocks.

I still don't really get it ;-).  You wrote an NTFS implementation
which is supposed to know about the disk layout of a file.  So why do
you need a hook to peek how NTFS works?  This sounds a bit circular to
me.

I am sure there is another and cleaner way to fix this.  This solution
can not be used inside a filesystem implementation because it might
break other code.  Can you please look for some other solution?

>> In this case the mtf->buf leaks, right?  Same for the returns below I
>> think when returning 0.
>
> The memory is released in free_file:
>
>   grub_free(mft->buf);

Ok.

--
Marco





reply via email to

[Prev in Thread] Current Thread [Next in Thread]