Re: [PATCH 7/7] split up xlog_recover_process_iunlinks

[Dave Chinner <david@xxxxxxxxxxxxx>]

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@xxxxxx>

.....

> 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@xxxxxxxxxxxxx