Re: hfs breakage

[Marco Gerards]

Bean <address@hidden> 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
> address@hidden
> http://lists.gnu.org/mailman/listinfo/grub-devel