Bean <[EMAIL PROTECTED]> 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 _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
