Re: [PATCH] Corrections to affs and sfs

[Robert Millan]

On Sat, Nov 22, 2008 at 08:55:19PM +0100, Krzysztof Smiechowicz wrote:
> Hello,
> 
> I would like to submit the following patch:
> 
> * fs/affs.c: Return failure when directory iteration failed.
> 
> * fs/sfs.c: Return failure when directory iteration failed. Correct
> order in which btree nodes are read.
> 
> These changes are needed to boot AROS Research Operating System from SFS
> / FFS.

Hi,

> Index: fs/affs.c
> ===================================================================
> --- fs/affs.c (revision 1919)
> +++ fs/affs.c (working copy)
> @@ -381,7 +381,7 @@
>   fail:
>    grub_free (node);
>    grub_free (hashtable);
> -  return 1;
> +  return 0;
>  }

I checked in this hunk (and the equivalent sfs.c one)

> Index: fs/sfs.c
> ===================================================================
> --- fs/sfs.c  (revision 1919)
> +++ fs/sfs.c  (working copy)
> @@ -172,7 +172,8 @@
>         return grub_errno;
>       }
>  
> -      for (i = 0; i < grub_be_to_cpu16 (tree->nodes); i++)
> +      grub_uint16_t nodescount = grub_be_to_cpu16(tree->nodes);
> +      for (i = nodescount - 1; i >= 0; i--)

nodescount is only used once; why adding a variable?

>         /* Follow the tree down to the leaf level.  */
> -       if ((grub_be_to_cpu32 (EXTNODE(tree, i)->key) >= block)
> +       if ((grub_be_to_cpu32 (EXTNODE(tree, i)->key) <= block)
>             && !tree->leaf)
>           {
> -           next = grub_be_to_cpu32 (EXTNODE (tree, i - 1)->data);
> -           break;
> -         }
> -
> -       /* In case the last node is reached just use that one, it is
> -          the right match.  */
> -       if (i + 1 == grub_be_to_cpu16 (tree->nodes) && !tree->leaf)
> -         {
>             next = grub_be_to_cpu32 (EXTNODE (tree, i)->data);
>             break;
>           }

I'm not familiar with our SFS code.  Marco, if you have a minute could you
review this part?

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."