Re: hfs breakage

[Marco Gerards]

Bean <address@hidden> writes:

> On Jan 21, 2008 6:38 PM, Marco Gerards <address@hidden> wrote:
>> Bean <address@hidden> writes:
>>
>> Hi Bean,
>>
>> Thanks for picking this one up!
>>
>> [...]
>>
>> >> Moreover, if I mount the image as loop in Linux and remove files "grub"
>> >> and "grub.cfg", "ls" in grub-emu will go into infinite loop and print
>> >> "ofboot.b pc.mod raid.mod reboot.mod reiserfs.mod search.mod sfs.mod
>> >> sun.mod suspend.mod terminal.mod" over and over again.
>> >
>> > I think i figure out the problem, please try the following patch.
>> >
>> >       * fs/hfs.c : Add magic values for cnid
>>
>> Did you forget the function name?
>
> perhaps i should use a name for the enum, something like hfs_cnid_t.

That might help :-)

Otherwise you have to list all the values in the changelog entry...

>> >       (grub_hfs_iterate_records): Use the correct file number for extents
>> > and catalog file. Fix problem in next index calculation.
>>
>> This line is too long, I think?  Can you wrap this manually?
>
> ok.

:-)

>>
>> >       (grub_hfs_find_node): Replace recursive function call with loop.
>> >       (grub_hfs_iterate_dir): Replace recursive function call with loop.
>>
>> :-)
>>
>> I guess this code sucked a bit ;-)
>
> is the fix ok ?

Yes, it seems so to me at least.  I was talking about my code :-)

>> > diff --git a/fs/hfs.c b/fs/hfs.c
>> > index e8e9c3e..3480d3e 100644
>> > --- a/fs/hfs.c
>> > +++ b/fs/hfs.c
>> > @@ -43,6 +43,16 @@ enum
>> >      GRUB_HFS_FILETYPE_FILE = 2
>> >    };
>> >
>> > +/* Catalog node ID (CNID).  */
>> > +enum
>> > +  {
>> > +    GRUB_HFS_CNID_ROOT_PARENT = 1,
>> > +    GRUB_HFS_CNID_ROOT = 2,
>> > +    GRUB_HFS_CNID_EXT = 3,
>> > +    GRUB_HFS_CNID_CAT = 4,
>> > +    GRUB_HFS_CNID_BAD = 5
>> > +  };
>>
>> This is missing in the changelog entry.
>>
>> >  /* A node descriptor.  This is the header of every node.  */
>> >  struct grub_hfs_node
>> >  {
>> > @@ -447,7 +457,8 @@ grub_hfs_iterate_records (struct grub_hfs_data
>> > *data, int type, int idx,
>> >
>> >        /* Read the node into memory.  */
>> >        blk = grub_hfs_block (data, dat,
>> > -                         0, idx / (data->blksz / nodesize), 0);
>> > +                            (type == 0) ? GRUB_HFS_CNID_CAT :
>> > GRUB_HFS_CNID_EXT,
>> > +                         idx / (data->blksz / nodesize), 0);
>>
>> What was the problem here?
>
> extents file and catalog file uses special file number, 3 and 4,
> respectively. for example, in sda.img, sector 5:
>
> 00 00 00 00 00 00 00 00 FF 01 00 01 00 00 07 00
> 00 00 00 04 00 2D 04 C3 00 0F 00 00 00 00 00 00
>
> this is the first extent of catalog file that's not stored in super
> block, you can see that it use 4 as the file number.

This applies in general?  I do not have a working mac around ATM to
test with...

--
Marco