Re: hfs breakage

[Bean]

On Jan 23, 2008 5:01 PM, Marco Gerards <address@hidden> wrote:
> 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_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