hfs breakage

hfs breakage

[Pavel Roskin]

Hello!

I have noticed that GRUB won't list all files on the boot partition of
my PowerMAC.  I can still get the full list if I mount the partition in
Linux of use hfsutils.

I put the image here:
http://red-bean.com/proski/sda2.img.bz2

It's just 210k compressed.  The problem can be reproduced on any
ordinary PC.  Just uncompress the image and create a file called
device.map with the following contents:

(hd0) sda2.img

Compile grub with grub-emu support:

./configure --enable-grub-emu && make

Run grub-emu:

grub-emu -m device.map -r hd0

grub> ls (hd0)/
acorn.mod affs.mod amiga.mod apple.mod blocklist.mod boot.mod cat.mod
cmp.mod configfile.mod cpio.mod elf.mod ext2.mod fat.mod font.mod
fshelp.mod gpt.mod grub
grub>

Files after "grub" are not shown.

I have already considered the possibility that having files with similar
names "grub" and "grub.cfg" may be a problem.  But renaming "grub" to
"gurb" makes no difference.

I have been rewriting the file called "grub" many times, so maybe it was
moved to some additional leaf of the directory tree, confusing the hfs
implementation in GRUB (I'm just guessing, I don't know anything about
hfs).

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.

-- 
Regards,
Pavel Roskin

Re: hfs breakage

[Bean]

On Jan 16, 2008 8:38 AM, Pavel Roskin <proski@gnu.org> wrote:
> Hello!
>
> I have noticed that GRUB won't list all files on the boot partition of
> my PowerMAC.  I can still get the full list if I mount the partition in
> Linux of use hfsutils.
>
> I put the image here:
> http://red-bean.com/proski/sda2.img.bz2
>
> It's just 210k compressed.  The problem can be reproduced on any
> ordinary PC.  Just uncompress the image and create a file called
> device.map with the following contents:
>
> (hd0) sda2.img
>
> Compile grub with grub-emu support:
>
> ./configure --enable-grub-emu && make
>
> Run grub-emu:
>
> grub-emu -m device.map -r hd0
>
> grub> ls (hd0)/
> acorn.mod affs.mod amiga.mod apple.mod blocklist.mod boot.mod cat.mod
> cmp.mod configfile.mod cpio.mod elf.mod ext2.mod fat.mod font.mod
> fshelp.mod gpt.mod grub
> grub>
>
> Files after "grub" are not shown.
>
> I have already considered the possibility that having files with similar
> names "grub" and "grub.cfg" may be a problem.  But renaming "grub" to
> "gurb" makes no difference.
>
> I have been rewriting the file called "grub" many times, so maybe it was
> moved to some additional leaf of the directory tree, confusing the hfs
> implementation in GRUB (I'm just guessing, I don't know anything about
> hfs).
>
> 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
	(grub_hfs_iterate_records): Use the correct file number for extents
and catalog file. Fix problem in next index calculation.
	(grub_hfs_find_node): Replace recursive function call with loop.
	(grub_hfs_iterate_dir): Replace recursive function call with loop.
	

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
+  };
+
 /* 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);
       blk += (idx % (data->blksz / nodesize));
       if (grub_errno)
 	return grub_errno;
@@ -481,10 +492,7 @@ grub_hfs_iterate_records (struct grub_hfs_data
*data, int type, int idx,
 	    return 0;
 	}

-      if (idx % (data->blksz / nodesize) == 0)
-	idx = grub_be_to_cpu32 (node.node.next);
-      else
-	idx++;
+      idx = grub_be_to_cpu32 (node.node.next);
     } while (idx && this);

   return 0;
@@ -501,6 +509,7 @@ grub_hfs_find_node (struct grub_hfs_data *data, char *key,
 {
   int found = -1;
   int isleaf = 0;
+  int done = 0;

   auto int node_found (struct grub_hfs_node *, struct grub_hfs_record *);

@@ -532,6 +541,8 @@ grub_hfs_find_node (struct grub_hfs_data *data, char *key,
 	  /* Found it!!!!  */
 	  if (cmp == 0)
 	    {
+              done = 1;
+
 	      grub_memcpy (datar, rec->data,
 			   rec->datalen < datalen ? rec->datalen : datalen);
 	      return 1;
@@ -541,16 +552,20 @@ grub_hfs_find_node (struct grub_hfs_data *data, char *key,
       return 0;
     }

-  if (grub_hfs_iterate_records (data, type, idx, 0, node_found))
-    return 0;
+  do
+    {
+      found = -1;
+
+      if (grub_hfs_iterate_records (data, type, idx, 0, node_found))
+        return 0;

-  if (found == -1)
-    return 0;
+      if (found == -1)
+        return 0;

-  if (isleaf)
-    return 1;
+      idx = found;
+    } while (! isleaf);

-  return grub_hfs_find_node (data, key, found, type, datar, datalen);
+  return done;
 }


@@ -607,21 +622,23 @@ grub_hfs_iterate_dir (struct grub_hfs_data
*data, grub_uint32_t root_idx,
       return hook (rec);
     }

-  if (grub_hfs_iterate_records (data, 0, root_idx, 0, node_found))
-    return grub_errno;
+  do
+    {
+      found = -1;
+
+      if (grub_hfs_iterate_records (data, 0, root_idx, 0, node_found))
+        return grub_errno;

-  if (found == -1)
-    return 0;
+      if (found == -1)
+        return 0;

+      root_idx = found;
+    } while (! isleaf);
+
   /* If there was a matching record in this leaf node, continue the
      iteration until the last record was found.  */
-  if (isleaf)
-    {
-      grub_hfs_iterate_records (data, 0, next, 1, it_dir);
-      return grub_errno;
-    }
-
-  return grub_hfs_iterate_dir (data, found, dir, hook);
+  grub_hfs_iterate_records (data, 0, next, 1, it_dir);
+  return grub_errno;
 }


-- 
Bean

Re: hfs breakage

[Pavel Roskin]

On Sat, 2008-01-19 at 15:12 +0800, Bean wrote:

> I think i figure out the problem, please try the following patch.

It's working.  Thank you!

-- 
Regards,
Pavel Roskin

Re: hfs breakage

[Marco Gerards]

Bean <bean123ch@gmail.com> 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?

> 	(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?

> 	(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 ;-)

> 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?

>        blk += (idx % (data->blksz / nodesize));
>        if (grub_errno)
>  	return grub_errno;
> @@ -481,10 +492,7 @@ grub_hfs_iterate_records (struct grub_hfs_data
> *data, int type, int idx,
>  	    return 0;
>  	}
>
> -      if (idx % (data->blksz / nodesize) == 0)
> -	idx = grub_be_to_cpu32 (node.node.next);
> -      else
> -	idx++;
> +      idx = grub_be_to_cpu32 (node.node.next);
>      } while (idx && this);
>
>    return 0;
> @@ -501,6 +509,7 @@ grub_hfs_find_node (struct grub_hfs_data *data, char *key,
>  {
>    int found = -1;
>    int isleaf = 0;
> +  int done = 0;
>
>    auto int node_found (struct grub_hfs_node *, struct grub_hfs_record *);
>
> @@ -532,6 +541,8 @@ grub_hfs_find_node (struct grub_hfs_data *data, char *key,
>  	  /* Found it!!!!  */
>  	  if (cmp == 0)
>  	    {
> +              done = 1;
> +
>  	      grub_memcpy (datar, rec->data,
>  			   rec->datalen < datalen ? rec->datalen : datalen);
>  	      return 1;
> @@ -541,16 +552,20 @@ grub_hfs_find_node (struct grub_hfs_data *data, char *key,
>        return 0;
>      }
>
> -  if (grub_hfs_iterate_records (data, type, idx, 0, node_found))
> -    return 0;
> +  do
> +    {
> +      found = -1;
> +
> +      if (grub_hfs_iterate_records (data, type, idx, 0, node_found))
> +        return 0;
>
> -  if (found == -1)
> -    return 0;
> +      if (found == -1)
> +        return 0;
>
> -  if (isleaf)
> -    return 1;
> +      idx = found;
> +    } while (! isleaf);
>
> -  return grub_hfs_find_node (data, key, found, type, datar, datalen);
> +  return done;
>  }
>
>
> @@ -607,21 +622,23 @@ grub_hfs_iterate_dir (struct grub_hfs_data
> *data, grub_uint32_t root_idx,
>        return hook (rec);
>      }
>
> -  if (grub_hfs_iterate_records (data, 0, root_idx, 0, node_found))
> -    return grub_errno;
> +  do
> +    {
> +      found = -1;
> +
> +      if (grub_hfs_iterate_records (data, 0, root_idx, 0, node_found))
> +        return grub_errno;
>
> -  if (found == -1)
> -    return 0;
> +      if (found == -1)
> +        return 0;
>
> +      root_idx = found;
> +    } while (! isleaf);
> +
>    /* If there was a matching record in this leaf node, continue the
>       iteration until the last record was found.  */
> -  if (isleaf)
> -    {
> -      grub_hfs_iterate_records (data, 0, next, 1, it_dir);
> -      return grub_errno;
> -    }
> -
> -  return grub_hfs_iterate_dir (data, found, dir, hook);
> +  grub_hfs_iterate_records (data, 0, next, 1, it_dir);
> +  return grub_errno;
>  }

Re: hfs breakage

[Bean]

On Jan 21, 2008 6:38 PM, Marco Gerards <mgerards@xs4all.nl> wrote:
> Bean <bean123ch@gmail.com> 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.

>
> >       (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 ?

>
> > 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.

-- 
Bean

Re: hfs breakage

[Marco Gerards]

Bean <bean123ch@gmail.com> writes:

> On Jan 21, 2008 6:38 PM, Marco Gerards <mgerards@xs4all.nl> wrote:
>> Bean <bean123ch@gmail.com> 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

Re: hfs breakage

[Bean]

On Jan 23, 2008 5:01 PM, Marco Gerards <mgerards@xs4all.nl> wrote:
> Bean <bean123ch@gmail.com> writes:
>
> > On Jan 21, 2008 6:38 PM, Marco Gerards <mgerards@xs4all.nl> wrote:
> >> Bean <bean123ch@gmail.com> 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_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 :-)

ok then, i just revert to the original code. however, there is still a
small bug in grub_hfs_find_node. please see the new patch below.

> >> 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...

i guess so, but i don't have mac to test on, maybe someone can verify it.

	* fs/hfs.c (grub_hfs_cnid_t): Add magic values for cnid
	(grub_hfs_iterate_records): Use the correct file number for extents
	and catalog file. Fix problem in next index calculation.
	(grub_hfs_find_node): Fix a bug that can cause the function to return
	1 even if no match is found.


diff --git a/fs/hfs.c b/fs/hfs.c
index e8e9c3e..f2afa8a 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
+  } grub_hfs_cnid_t;
+
 /* 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);
       blk += (idx % (data->blksz / nodesize));
       if (grub_errno)
 	return grub_errno;
@@ -481,10 +492,7 @@ grub_hfs_iterate_records (struct grub_hfs_data
*data, int type, int idx,
 	    return 0;
 	}

-      if (idx % (data->blksz / nodesize) == 0)
-	idx = grub_be_to_cpu32 (node.node.next);
-      else
-	idx++;
+      idx = grub_be_to_cpu32 (node.node.next);
     } while (idx && this);

   return 0;
@@ -501,6 +509,7 @@ grub_hfs_find_node (struct grub_hfs_data *data, char *key,
 {
   int found = -1;
   int isleaf = 0;
+  int done = 0;

   auto int node_found (struct grub_hfs_node *, struct grub_hfs_record *);

@@ -532,6 +541,8 @@ grub_hfs_find_node (struct grub_hfs_data *data, char *key,
 	  /* Found it!!!!  */
 	  if (cmp == 0)
 	    {
+              done = 1;
+
 	      grub_memcpy (datar, rec->data,
 			   rec->datalen < datalen ? rec->datalen : datalen);
 	      return 1;
@@ -548,7 +559,7 @@ grub_hfs_find_node (struct grub_hfs_data *data, char *key,
     return 0;

   if (isleaf)
-    return 1;
+    return done;

   return grub_hfs_find_node (data, key, found, type, datar, datalen);
 }


-- 
Bean

Re: hfs breakage

[Marco Gerards]

Bean <bean123ch@gmail.com> writes:

[...]

>> Yes, it seems so to me at least.  I was talking about my code :-)
>
> ok then, i just revert to the original code. however, there is still a
> small bug in grub_hfs_find_node. please see the new patch below.

No need to revert it, you improved it!

>> >> 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...
>
> i guess so, but i don't have mac to test on, maybe someone can verify it.

:-)

> 	* fs/hfs.c (grub_hfs_cnid_t): Add magic values for cnid

_t is for types, can you prefix this with "type"?

So this would become:

 	* fs/hfs.c (grub_hfs_cnid_t): New type.

> 	(grub_hfs_iterate_records): Use the correct file number for extents
> 	and catalog file. Fix problem in next index calculation.

". " -> ".  "

> 	(grub_hfs_find_node): Fix a bug that can cause the function to return
> 	1 even if no match is found.


> +/* 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
> +  } grub_hfs_cnid_t;

+ type

>  /* 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);
>        blk += (idx % (data->blksz / nodesize));
>        if (grub_errno)
>  	return grub_errno;
> @@ -481,10 +492,7 @@ grub_hfs_iterate_records (struct grub_hfs_data
> *data, int type, int idx,
>  	    return 0;
>  	}
>
> -      if (idx % (data->blksz / nodesize) == 0)
> -	idx = grub_be_to_cpu32 (node.node.next);
> -      else
> -	idx++;
> +      idx = grub_be_to_cpu32 (node.node.next);
>      } while (idx && this);
>
>    return 0;
> @@ -501,6 +509,7 @@ grub_hfs_find_node (struct grub_hfs_data *data, char *key,
>  {
>    int found = -1;
>    int isleaf = 0;
> +  int done = 0;
>
>    auto int node_found (struct grub_hfs_node *, struct grub_hfs_record *);
>
> @@ -532,6 +541,8 @@ grub_hfs_find_node (struct grub_hfs_data *data, char *key,
>  	  /* Found it!!!!  */
>  	  if (cmp == 0)
>  	    {
> +              done = 1;
> +
>  	      grub_memcpy (datar, rec->data,
>  			   rec->datalen < datalen ? rec->datalen : datalen);
>  	      return 1;
> @@ -548,7 +559,7 @@ grub_hfs_find_node (struct grub_hfs_data *data, char *key,
>      return 0;
>
>    if (isleaf)
> -    return 1;
> +    return done;
>
>    return grub_hfs_find_node (data, key, found, type, datar, datalen);
>  }
>
>
> -- 
> Bean
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel

Re: hfs breakage

[Bean]

Committed.

-- 
Bean