Date: Thu, 21 Apr 2011 00:52:03 -0400
From: Christoph Hellwig <hch@infradead.org>
To: Dave Chinner <david@fromorbit.com>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH 3/4] xfs: reset buffer pointers before freeing them
Message-ID: <20110421045203.GB28859@infradead.org>
References: <1303360144-10632-1-git-send-email-david@fromorbit.com>
 <1303360144-10632-4-git-send-email-david@fromorbit.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1303360144-10632-4-git-send-email-david@fromorbit.com>
User-Agent: Mutt/1.5.21 (2010-09-15)

>  		ealign = round_down(end_block, sectbb);
>  		if (j == 0 && (start_block + endcount > ealign)) {
> -			offset = XFS_BUF_PTR(bp);
> -			balign = BBTOB(ealign - start_block);
> -			error = XFS_BUF_SET_PTR(bp, offset + balign,
> -						BBTOB(sectbb));
> +			balign = ealign - start_block;
> +			offset = XFS_BUF_PTR(bp) + BBTOB(balign);
> +			error = xlog_bread_offset(log, ealign, sectbb,
> +							bp, offset);

I'd remove the use of balign entirely here.  The first use of this
variable earlier in the functions is for something entirely different,
so it's rather confusing.  (I only looked into that because I remember
some align variable beeing used later on, but that was elsewhere)

Otherwise looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>