Re: hfs breakage

[Marco Gerards <mgerards@xs4all.nl>]

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