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@xxxxxxxxxxxxx
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
