Re: [PATCH] xfs: reset buffer pointers before freeing them

[Dave Chinner <david@fromorbit.com>]

On Tue, Apr 12, 2011 at 12:22:37PM -0400, Christoph Hellwig wrote:
> On Tue, Apr 12, 2011 at 11:45:43PM +1000, Dave Chinner wrote:
> > Something like this?
> 
> Kinda.  A few comments below.
> 
> >  /*
> > + * Read at an offset into the buffer. Returns with the buffer in it's original
> > + * state regardless of the result of the read.
> > + */
> > +STATIC int
> > +xlog_bread_offset(
> > +	xlog_t		*log,
> > +	int		buf_offset,	/* offset into buffer */
> > +	int		buf_blks,	/* original buffer size */
> > +	xfs_daddr_t	blk_no,		/* block to read from */
> > +	int		nbblks,		/* blocks to read */
> > +	xfs_buf_t	*bp,
> > +	xfs_caddr_t	offset)
> > +{
> > +	xfs_caddr_t	orig_offset = XFS_BUF_PTR(bp);
> > +	int		error, error2;
> > +
> > +	error = XFS_BUF_SET_PTR(bp, offset + BBTOB(buf_offset), BBTOB(nbblks));
> 
> Passing in both offset and the buf_offset seems odd as they are only
> used together.  So just passing in one byte offset would make more
> sense to me.  Of course it's not actually an offset anyore, but virtual
> address, but we use that offset naming all around xlog_align/xlog_bread
> and callers.

Fair enough. I was struggling with the best way to pass and name
all the parameters. I just posted this before I went to bed to see
if this is what you had in mind....

> Also I don't think there is a need for the buf_blks argument.  The
> argument always comes directly from the xlog_get_bp number blocks
> argument, so the
> 
> 	BBTOB(buf_blks)
> 
> can be replaced by simply using a copy of b_buffer_length taken at
> the beginning of the function.

Very true. My brain surely was not functioning properly ;)

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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