Re: [PATCH 4/4] xfs: remove rbpp check from xfs_rtmodify_summary_int

[Brian Foster <bfoster@xxxxxxxxxx>]

On Thu, Aug 21, 2014 at 08:03:19PM -0500, Eric Sandeen wrote:
> rbpp is always passed into xfs_rtmodify_summary
> and xfs_rtget_summary, so there is no need to
> test for it in xfs_rtmodify_summary_int.
> 

Looks fine, but this is also called through a variety of twisty paths.
Could we add a top-level error check or assert?

Brian

> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx>
> ---
> 
> diff --git a/fs/xfs/libxfs/xfs_rtbitmap.c b/fs/xfs/libxfs/xfs_rtbitmap.c
> index 50e3b93..7c818f1 100644
> --- a/fs/xfs/libxfs/xfs_rtbitmap.c
> +++ b/fs/xfs/libxfs/xfs_rtbitmap.c
> @@ -460,7 +460,7 @@ xfs_rtmodify_summary_int(
>       /*
>        * If we have an old buffer, and the block number matches, use that.
>        */
> -     if (rbpp && *rbpp && *rsb == sb)
> +     if (*rbpp && *rsb == sb)
>               bp = *rbpp;
>       /*
>        * Otherwise we have to get the buffer.
> @@ -469,7 +469,7 @@ xfs_rtmodify_summary_int(
>               /*
>                * If there was an old one, get rid of it first.
>                */
> -             if (rbpp && *rbpp)
> +             if (*rbpp)
>                       xfs_trans_brelse(tp, *rbpp);
>               error = xfs_rtbuf_get(mp, tp, sb, 1, &bp);
>               if (error) {
> @@ -478,10 +478,8 @@ xfs_rtmodify_summary_int(
>               /*
>                * Remember this buffer and block for the next call.
>                */
> -             if (rbpp) {
> -                     *rbpp = bp;
> -                     *rsb = sb;
> -             }
> +             *rbpp = bp;
> +             *rsb = sb;
>       }
>       /*
>        * Point to the summary information, modify/log it, and/or copy it out.
> @@ -493,14 +491,8 @@ xfs_rtmodify_summary_int(
>               *sp += delta;
>               xfs_trans_log_buf(tp, bp, first, first + sizeof(*sp) - 1);
>       }
> -     if (sum) {
> -             /*
> -              * Drop the buffer if we're not asked to remember it.
> -              */
> -             if (!rbpp)
> -                     xfs_trans_brelse(tp, bp);
> +     if (sum)
>               *sum = *sp;
> -     }
>       return 0;
>  }
>  
> 
> 
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs