* [PATCH 0/2] Check return value of xfs_buf_read() @ 2011-08-03 2:18 Chandra Seetharaman 2011-08-03 2:18 ` [PATCH 1/2] xfs: Check the return value of xfs_buf_read() for NULL Chandra Seetharaman 2011-08-03 2:18 ` [PATCH 2/2] xfs: replace xfs_buf_geterror() with bp->b_error Chandra Seetharaman 0 siblings, 2 replies; 5+ messages in thread From: Chandra Seetharaman @ 2011-08-03 2:18 UTC (permalink / raw) To: xfs; +Cc: Chandra Seetharaman Hello All, While doing the previous clean up Alex noted that some of the places where xfs_buf_read() returns, the return value is not checked for NULL. This set check for the return value and returns failure if NULL and also avoids using xfs_buf_geterror() when not needed. Thanks & Regards, chandra _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] xfs: Check the return value of xfs_buf_read() for NULL 2011-08-03 2:18 [PATCH 0/2] Check return value of xfs_buf_read() Chandra Seetharaman @ 2011-08-03 2:18 ` Chandra Seetharaman 2011-08-11 22:04 ` Alex Elder 2011-08-03 2:18 ` [PATCH 2/2] xfs: replace xfs_buf_geterror() with bp->b_error Chandra Seetharaman 1 sibling, 1 reply; 5+ messages in thread From: Chandra Seetharaman @ 2011-08-03 2:18 UTC (permalink / raw) To: xfs; +Cc: Chandra Seetharaman Check the return value of xfs_buf_read() for NULL and return ENOMEM if it is NULL. Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com> --- fs/xfs/xfs_log_recover.c | 6 ++++++ fs/xfs/xfs_rw.c | 2 +- fs/xfs/xfs_vnodeops.c | 2 ++ 3 files changed, 9 insertions(+), 1 deletions(-) diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c index 93786e5..8478532 100644 --- a/fs/xfs/xfs_log_recover.c +++ b/fs/xfs/xfs_log_recover.c @@ -2131,6 +2131,8 @@ xlog_recover_buffer_pass2( bp = xfs_buf_read(mp->m_ddev_targp, buf_f->blf_blkno, buf_f->blf_len, buf_flags); + if (!bp) + return XFS_ERROR(ENOMEM); error = xfs_buf_geterror(bp); if (error) { xfs_ioerror_alert("xlog_recover_do..(read#1)", mp, @@ -2222,6 +2224,10 @@ xlog_recover_inode_pass2( bp = xfs_buf_read(mp->m_ddev_targp, in_f->ilf_blkno, in_f->ilf_len, XBF_LOCK); + if (!bp) { + error = ENOMEM; + goto error; + } error = xfs_buf_geterror(bp); if (error) { xfs_ioerror_alert("xlog_recover_do..(read#2)", mp, diff --git a/fs/xfs/xfs_rw.c b/fs/xfs/xfs_rw.c index c96a8a0..ecaa60e 100644 --- a/fs/xfs/xfs_rw.c +++ b/fs/xfs/xfs_rw.c @@ -136,7 +136,7 @@ xfs_read_buf( bp = xfs_buf_read(target, blkno, len, flags); if (!bp) - return XFS_ERROR(EIO); + return (flags & XBF_TRYLOCK) ? 0 : XFS_ERROR(EIO); error = bp->b_error; if (!error && !XFS_FORCED_SHUTDOWN(mp)) { *bpp = bp; diff --git a/fs/xfs/xfs_vnodeops.c b/fs/xfs/xfs_vnodeops.c index 3ee5f8a..64a5835 100644 --- a/fs/xfs/xfs_vnodeops.c +++ b/fs/xfs/xfs_vnodeops.c @@ -83,6 +83,8 @@ xfs_readlink_bmap( bp = xfs_buf_read(mp->m_ddev_targp, d, BTOBB(byte_cnt), XBF_LOCK | XBF_MAPPED | XBF_DONT_BLOCK); + if (!bp) + return XFS_ERROR(ENOMEM); error = xfs_buf_geterror(bp); if (error) { xfs_ioerror_alert("xfs_readlink", -- 1.7.1 _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] xfs: Check the return value of xfs_buf_read() for NULL 2011-08-03 2:18 ` [PATCH 1/2] xfs: Check the return value of xfs_buf_read() for NULL Chandra Seetharaman @ 2011-08-11 22:04 ` Alex Elder 0 siblings, 0 replies; 5+ messages in thread From: Alex Elder @ 2011-08-11 22:04 UTC (permalink / raw) To: Chandra Seetharaman; +Cc: xfs On Tue, 2011-08-02 at 19:18 -0700, Chandra Seetharaman wrote: > Check the return value of xfs_buf_read() for NULL and return ENOMEM if it is NULL. You might add that the reason for doing this is that in these spots the null buffer pointer would be blindly dereferenced otherwise. One piece of this patch needs to go, so please post an updated version. But you can consider this reviewed by me. Reviewed-by: Alex Elder <aelder@sgi.com> > Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com> > --- > fs/xfs/xfs_log_recover.c | 6 ++++++ > fs/xfs/xfs_rw.c | 2 +- > fs/xfs/xfs_vnodeops.c | 2 ++ > 3 files changed, 9 insertions(+), 1 deletions(-) > > diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c > index 93786e5..8478532 100644 > --- a/fs/xfs/xfs_log_recover.c > +++ b/fs/xfs/xfs_log_recover.c > @@ -2131,6 +2131,8 @@ xlog_recover_buffer_pass2( > > bp = xfs_buf_read(mp->m_ddev_targp, buf_f->blf_blkno, buf_f->blf_len, > buf_flags); > + if (!bp) > + return XFS_ERROR(ENOMEM); > error = xfs_buf_geterror(bp); > if (error) { > xfs_ioerror_alert("xlog_recover_do..(read#1)", mp, > @@ -2222,6 +2224,10 @@ xlog_recover_inode_pass2( > > bp = xfs_buf_read(mp->m_ddev_targp, in_f->ilf_blkno, in_f->ilf_len, > XBF_LOCK); > + if (!bp) { > + error = ENOMEM; > + goto error; > + } > error = xfs_buf_geterror(bp); > if (error) { > xfs_ioerror_alert("xlog_recover_do..(read#2)", mp, > diff --git a/fs/xfs/xfs_rw.c b/fs/xfs/xfs_rw.c > index c96a8a0..ecaa60e 100644 > --- a/fs/xfs/xfs_rw.c > +++ b/fs/xfs/xfs_rw.c > @@ -136,7 +136,7 @@ xfs_read_buf( > > bp = xfs_buf_read(target, blkno, len, flags); > if (!bp) > - return XFS_ERROR(EIO); > + return (flags & XBF_TRYLOCK) ? 0 : XFS_ERROR(EIO); This change doesn't belong in this patch. As far as I can tell, no call to xfs_read_buf() ever passes XBF_TRYLOCK in flags anyway (only XBF_LOCK, XBF_DONT_BLOCK, and XBF_MAPPED). > error = bp->b_error; > if (!error && !XFS_FORCED_SHUTDOWN(mp)) { > *bpp = bp; > diff --git a/fs/xfs/xfs_vnodeops.c b/fs/xfs/xfs_vnodeops.c > index 3ee5f8a..64a5835 100644 > --- a/fs/xfs/xfs_vnodeops.c > +++ b/fs/xfs/xfs_vnodeops.c > @@ -83,6 +83,8 @@ xfs_readlink_bmap( > > bp = xfs_buf_read(mp->m_ddev_targp, d, BTOBB(byte_cnt), > XBF_LOCK | XBF_MAPPED | XBF_DONT_BLOCK); > + if (!bp) > + return XFS_ERROR(ENOMEM); > error = xfs_buf_geterror(bp); > if (error) { > xfs_ioerror_alert("xfs_readlink", _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/2] xfs: replace xfs_buf_geterror() with bp->b_error 2011-08-03 2:18 [PATCH 0/2] Check return value of xfs_buf_read() Chandra Seetharaman 2011-08-03 2:18 ` [PATCH 1/2] xfs: Check the return value of xfs_buf_read() for NULL Chandra Seetharaman @ 2011-08-03 2:18 ` Chandra Seetharaman 2011-08-11 22:04 ` Alex Elder 1 sibling, 1 reply; 5+ messages in thread From: Chandra Seetharaman @ 2011-08-03 2:18 UTC (permalink / raw) To: xfs; +Cc: Chandra Seetharaman Since we just checked bp for NULL, it is ok to replace xfs_buf_geterror() with bp->b_error in these places. Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com> --- fs/xfs/xfs_log_recover.c | 4 ++-- fs/xfs/xfs_vnodeops.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c index 8478532..a9df667 100644 --- a/fs/xfs/xfs_log_recover.c +++ b/fs/xfs/xfs_log_recover.c @@ -2133,7 +2133,7 @@ xlog_recover_buffer_pass2( buf_flags); if (!bp) return XFS_ERROR(ENOMEM); - error = xfs_buf_geterror(bp); + error = bp->b_error; if (error) { xfs_ioerror_alert("xlog_recover_do..(read#1)", mp, bp, buf_f->blf_blkno); @@ -2228,7 +2228,7 @@ xlog_recover_inode_pass2( error = ENOMEM; goto error; } - error = xfs_buf_geterror(bp); + error = bp->b_error; if (error) { xfs_ioerror_alert("xlog_recover_do..(read#2)", mp, bp, in_f->ilf_blkno); diff --git a/fs/xfs/xfs_vnodeops.c b/fs/xfs/xfs_vnodeops.c index 64a5835..d465c94 100644 --- a/fs/xfs/xfs_vnodeops.c +++ b/fs/xfs/xfs_vnodeops.c @@ -85,7 +85,7 @@ xfs_readlink_bmap( XBF_LOCK | XBF_MAPPED | XBF_DONT_BLOCK); if (!bp) return XFS_ERROR(ENOMEM); - error = xfs_buf_geterror(bp); + error = bp->b_error; if (error) { xfs_ioerror_alert("xfs_readlink", ip->i_mount, bp, XFS_BUF_ADDR(bp)); -- 1.7.1 _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] xfs: replace xfs_buf_geterror() with bp->b_error 2011-08-03 2:18 ` [PATCH 2/2] xfs: replace xfs_buf_geterror() with bp->b_error Chandra Seetharaman @ 2011-08-11 22:04 ` Alex Elder 0 siblings, 0 replies; 5+ messages in thread From: Alex Elder @ 2011-08-11 22:04 UTC (permalink / raw) To: Chandra Seetharaman; +Cc: xfs On Tue, 2011-08-02 at 19:18 -0700, Chandra Seetharaman wrote: > Since we just checked bp for NULL, it is ok to replace xfs_buf_geterror() > with bp->b_error in these places. > > Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com> Looks good. Reviewed-by: Alex Elder <aelder@sgi.com> _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-08-11 22:04 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-08-03 2:18 [PATCH 0/2] Check return value of xfs_buf_read() Chandra Seetharaman 2011-08-03 2:18 ` [PATCH 1/2] xfs: Check the return value of xfs_buf_read() for NULL Chandra Seetharaman 2011-08-11 22:04 ` Alex Elder 2011-08-03 2:18 ` [PATCH 2/2] xfs: replace xfs_buf_geterror() with bp->b_error Chandra Seetharaman 2011-08-11 22:04 ` Alex Elder
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox