Re: [PATCH 03/11] xfs: factor out xfs_dir2_leaf_find_stale

Dave Chinner
2011-07-13T06:49:42Z

On Mon, Jul 11, 2011 at 05:32:53PM -0500, Alex Elder wrote:
> On Sun, 2011-07-10 at 16:49 -0400, Christoph Hellwig wrote:
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
>
> OK, I guess this is fine. I have a suggestion below
> but I see nothing truly wrong with what you have.
>
> Reviewed-by: Alex Elder <aelder@sgi.com>
>
> > Index: xfs/fs/xfs/xfs_dir2_leaf.c
> > ===================================================================
> > --- xfs.orig/fs/xfs/xfs_dir2_leaf.c 2011-07-09 12:07:49.518499801 +0200
> > +++ xfs/fs/xfs/xfs_dir2_leaf.c 2011-07-09 12:19:48.171796748 +0200
> > @@ -148,6 +148,36 @@ xfs_dir2_block_to_leaf(
> > return 0;
> > }
> >
> > +STATIC void
> > +xfs_dir2_leaf_find_stale(
> > + struct xfs_dir2_leaf *leaf,
> > + int index,
> > + int *lowstale,
> > + int *highstale)
> > +{
> > + /*
> > + * Find the first stale entry before our index, if any.
> > + */
> > + for (*lowstale = index - 1;
> > + *lowstale >= 0 &&
> > + leaf->ents[*lowstale].address !=
> > + cpu_to_be32(XFS_DIR2_NULL_DATAPTR);
> > + --*lowstale)
> > + continue;

Only thing I was conerned about was the indenting on these loops.
Something like this:

for (*lowstale = index - 1;
*lowstale >= 0 &&
leaf->ents[*lowstale].address !=
cpu_to_be32(XFS_DIR2_NULL_DATAPTR);
--*lowstale)
continue;

means that at a glance it is easy to separate the loop control
statements from the body of the loop just by indentation.

Cheers,

Dave.
--
Dave Chinner
david@fromorbit.com