Re: [PATCHv2 5/5] xfs: kill off l_sectbb_mask

[Dave Chinner <david@fromorbit.com>]

On Fri, Apr 16, 2010 at 03:54:05PM -0500, Alex Elder wrote:
> There remains only one user of the l_sectbb_mask field in the log
> structure.  Just kill it off and compute the mask where needed from
> the power-of-2 sector size.
> 
> (Only update from last post is to accomodate the changes in the
> previous patch in the series.)
> 
> Signed-off-by: Alex Elder <aelder@sgi.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
>  	xfs_buf_t	*bp)
>  {
> +	xfs_daddr_t	offset;
>  	xfs_caddr_t	ptr;
>  
> -	if (log->l_sectBBsize == 1)
> -		return XFS_BUF_PTR(bp);
> +	offset = blk_no & ((xfs_daddr_t) log->l_sectBBsize - 1);
> +	ptr = XFS_BUF_PTR(bp) + BBTOB(offset);
> +
> +	ASSERT(ptr + BBTOB(nbblks) <= XFS_BUF_PTR(bp) + XFS_BUF_SIZE(bp));

The ASSERT is more obfuscated than it needs to be. It's not obvious
that it is bounds checking offset+nbblks. i.e. I prefer the original
format like:

	ASSERT(BBTOB(offset + nbblks) <= XFS_BUF_SIZE(bp));

but otherwise it looks good. Anyway, minor nitpick, so:

Reviewed-by: Dave Chinner <david@fromorbit.com>

-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs