Re: [PATCH] Corrections to affs and sfs

[Robert Millan <rmh@aybabtu.com>]

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