Re: NTFS file system driver (update 3)

[Marco Gerards]

Bean <address@hidden> writes:

> On 7/31/07, Marco Gerards <address@hidden> wrote:
>> Bean <address@hidden> writes:
>>
>> > On Mon, Jul 30, 2007 at 01:19:32PM +0200, Marco Gerards wrote:
>> >> Can you pass it around inside one of the structs?  I understand that
>> >> passing it around as parameter is not really nice.  The problem is
>> >> that this might actually break other code, which I want to prevent.  I
>> >> hope you see this might become a serious problem?
>> >
>> > Ok, I pass the hook around using parameter, please see if there is other
>> > problem.
>>
>> It looks like you still use the read hook, as in you change the read
>> hook of the disk, am I right?  With passing "it" around I meant some
>> hook, not the disk-> hook.  Because this is more like a higher level
>> function, using this in the filesystem has some problems like I will
>> describe below.
>>
>> I noticed in your other email that you were playing with the blocklist
>> command.  Did you check if the blocklist that is returned by the
>> blocklist command is *exactly* as it is supposed to be?  So that no
>> blocks are missing or added because of your code?
>>
>> My main problem is this...  You do not need a hook for this.  Can you
>> please in detail describe what the hook is supposed to do?  The main
>> reason why I ask about this is that you do not need the hook *at all*.
>> It makes the code very hard to understand and I am, as you noticed,
>> afraid that it might break the blocklist feature.
>>
>> If you need a block number for certain data, you can better make a
>> function to calculate the block number somehow.  Or store it, or
>> whatever.  I am sure you would use something like this if there was no
>> hook, so can't you just pretend that this hook does not exist?
>>
>> If something is not clear yet, feel free to ask me.  My problem is
>> that I can not fully understand the code yet.  But if you show me
>> where to look and tell me what certain things do, I am certainly
>> willing to invest some time in this.
>
> I believe disk hook is ok, as in grub_fshelp_read_file:
>
> if (blknr)
>         {
>           disk->read_hook = read_hook;
>
>           grub_disk_read (disk, blknr, skipfirst,
>                           blockend, buf);
>           disk->read_hook = 0;
>           if (grub_errno)
>             return -1;
>         }
>
> grub_disk_read will call the hook when it's appropriate.

Ok.

> btw, The blocklist command works for files in ntfs partition.

Ok, that things will not be broken is the most important thing to me.
I do not really like the use of hooks since that makes the code a bit
hard to maintain for me.  In case of bug reports, I hope you will be
available to handle them?

Later today I will go over your patch and apply it.

Thanks,
Marco