Re: [PATCH 7/7] split up xlog_recover_process_iunlinks

[Dave Chinner <david@fromorbit.com>]

On Mon, Oct 27, 2008 at 09:39:38AM -0400, Christoph Hellwig wrote:
> Split out the body of the main loop into a separate helper to make the
> code readable.
> 
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

.....

> Index: linux-2.6-xfs/fs/xfs/xfs_log_recover.c
> ===================================================================
> --- linux-2.6-xfs.orig/fs/xfs/xfs_log_recover.c	2008-10-25 13:22:29.000000000 +0200
> +++ linux-2.6-xfs/fs/xfs/xfs_log_recover.c	2008-10-25 13:26:44.000000000 +0200
> @@ -3147,6 +3147,70 @@ out_error:
>  	return;
>  }
>  
> +STATIC xfs_agino_t
> +xlog_recover_process_one_iunlink(
> +	struct xfs_mount		*mp,
> +	xfs_agnumber_t			agno,
> +	xfs_agino_t			agino,
> +	int				bucket)
> +{
> +	struct xfs_buf			*ibp;
> +	struct xfs_dinode		*dip;
> +	struct xfs_inode		*ip;
> +	xfs_ino_t			ino;
> +	int				error;
> +
> +	ino = XFS_AGINO_TO_INO(mp, agno, agino);
> +	error = xfs_iget(mp, NULL, ino, 0, 0, &ip, 0);
> +	if (error)
> +		goto fail;
> +
> +	/*
> +	 * Get the on disk inode to find the next inode in the bucket.
> +	 */
> +	ASSERT(ip != NULL);
> +	error = xfs_itobp(mp, NULL, ip, &dip, &ibp, 0, 0, XFS_BUF_LOCK);
> +	if (error)
> +		goto fail;

Jumping to 'fail' at this point leaks the xfs_inode returned from
xfs_iget(). Hmmmm - it appears the original code leaked too....

Also, I don't think we need the assert for ip, either.  If it's NULL
then we'll panic immediately on entering xfs_itobp().

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com