Variable 'err' set but not used.
fs/buffer.c:2613:2: warning: Value stored to 'err' is never read.
Reported-by: Abaci Robot <[email protected]>
Link: https://bugzilla.openanolis.cn/show_bug.cgi?id=4589
Signed-off-by: Jiapeng Chong <[email protected]>
---
Changes in v2:
-Remove unused out label.
fs/buffer.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/fs/buffer.c b/fs/buffer.c
index d759b105c1e7..b3eb905f87d6 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -2580,7 +2580,7 @@ int block_truncate_page(struct address_space *mapping,
struct inode *inode = mapping->host;
struct page *page;
struct buffer_head *bh;
- int err;
+ int err = 0;
blocksize = i_blocksize(inode);
length = offset & (blocksize - 1);
@@ -2593,9 +2593,8 @@ int block_truncate_page(struct address_space *mapping,
iblock = (sector_t)index << (PAGE_SHIFT - inode->i_blkbits);
page = grab_cache_page(mapping, index);
- err = -ENOMEM;
if (!page)
- goto out;
+ return -ENOMEM;
if (!page_has_buffers(page))
create_empty_buffers(page, blocksize, 0);
@@ -2609,7 +2608,6 @@ int block_truncate_page(struct address_space *mapping,
pos += blocksize;
}
- err = 0;
if (!buffer_mapped(bh)) {
WARN_ON(bh->b_size != blocksize);
err = get_block(inode, iblock, bh, 0);
@@ -2633,12 +2631,11 @@ int block_truncate_page(struct address_space *mapping,
zero_user(page, offset, length);
mark_buffer_dirty(bh);
- err = 0;
unlock:
unlock_page(page);
put_page(page);
-out:
+
return err;
}
EXPORT_SYMBOL(block_truncate_page);
--
2.20.1.7.g153144c
On Thu, 23 Mar 2023 10:32:59 +0800, Jiapeng Chong wrote:
> Variable 'err' set but not used.
>
> fs/buffer.c:2613:2: warning: Value stored to 'err' is never read.
>
>
Applied to
tree: git://git.kernel.org/pub/scm/linux/kernel/git/vfs/idmapping.git
branch: fs.misc
[1/1] fs/buffer: Remove redundant assignment to err
commit: dc7cb2d29805fe4fa4000fc0b09740fc24c93408
Thanks!
Christian
On Thu 23-03-23 10:32:59, Jiapeng Chong wrote:
> Variable 'err' set but not used.
>
> fs/buffer.c:2613:2: warning: Value stored to 'err' is never read.
>
> Reported-by: Abaci Robot <[email protected]>
> Link: https://bugzilla.openanolis.cn/show_bug.cgi?id=4589
> Signed-off-by: Jiapeng Chong <[email protected]>
I don't think the patch is quite correct (Christian, please drop it if I'm
correct). See below:
> diff --git a/fs/buffer.c b/fs/buffer.c
> index d759b105c1e7..b3eb905f87d6 100644
> --- a/fs/buffer.c
> +++ b/fs/buffer.c
> @@ -2580,7 +2580,7 @@ int block_truncate_page(struct address_space *mapping,
> struct inode *inode = mapping->host;
> struct page *page;
> struct buffer_head *bh;
> - int err;
> + int err = 0;
>
> blocksize = i_blocksize(inode);
> length = offset & (blocksize - 1);
> @@ -2593,9 +2593,8 @@ int block_truncate_page(struct address_space *mapping,
> iblock = (sector_t)index << (PAGE_SHIFT - inode->i_blkbits);
>
> page = grab_cache_page(mapping, index);
> - err = -ENOMEM;
> if (!page)
> - goto out;
> + return -ENOMEM;
>
> if (!page_has_buffers(page))
> create_empty_buffers(page, blocksize, 0);
> @@ -2609,7 +2608,6 @@ int block_truncate_page(struct address_space *mapping,
> pos += blocksize;
> }
>
> - err = 0;
> if (!buffer_mapped(bh)) {
> WARN_ON(bh->b_size != blocksize);
> err = get_block(inode, iblock, bh, 0);
> @@ -2633,12 +2631,11 @@ int block_truncate_page(struct address_space *mapping,
>
> zero_user(page, offset, length);
> mark_buffer_dirty(bh);
> - err = 0;
There is:
if (!buffer_uptodate(bh) && !buffer_delay(bh) && !buffer_unwritten(bh))
err = -EIO;
above this assignment. So now we'll be returning -EIO if
block_truncate_page() needs to read the block AFAICT. Did this pass fstests
with some filesystem exercising this code (ext2 driver comes to mind)?
Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR
On Mon, Mar 27, 2023 at 10:10:10AM +0200, Christian Brauner wrote:
>
> On Thu, 23 Mar 2023 10:32:59 +0800, Jiapeng Chong wrote:
> > Variable 'err' set but not used.
> >
> > fs/buffer.c:2613:2: warning: Value stored to 'err' is never read.
> >
> >
>
> Applied to
I think you should exercise extreme care with patches from "Abaci Robot".
It's wrong more often than it's right, and the people interpreting the
output from it do not appear to be experienced programmers.
On Mon, Apr 03, 2023 at 06:10:43PM +0200, Jan Kara wrote:
> On Thu 23-03-23 10:32:59, Jiapeng Chong wrote:
> > Variable 'err' set but not used.
> >
> > fs/buffer.c:2613:2: warning: Value stored to 'err' is never read.
> >
> > Reported-by: Abaci Robot <[email protected]>
> > Link: https://bugzilla.openanolis.cn/show_bug.cgi?id=4589
> > Signed-off-by: Jiapeng Chong <[email protected]>
>
> I don't think the patch is quite correct (Christian, please drop it if I'm
> correct). See below:
Thank you for taking a look at this!
>
> > diff --git a/fs/buffer.c b/fs/buffer.c
> > index d759b105c1e7..b3eb905f87d6 100644
> > --- a/fs/buffer.c
> > +++ b/fs/buffer.c
> > @@ -2580,7 +2580,7 @@ int block_truncate_page(struct address_space *mapping,
> > struct inode *inode = mapping->host;
> > struct page *page;
> > struct buffer_head *bh;
> > - int err;
> > + int err = 0;
> >
> > blocksize = i_blocksize(inode);
> > length = offset & (blocksize - 1);
> > @@ -2593,9 +2593,8 @@ int block_truncate_page(struct address_space *mapping,
> > iblock = (sector_t)index << (PAGE_SHIFT - inode->i_blkbits);
> >
> > page = grab_cache_page(mapping, index);
> > - err = -ENOMEM;
> > if (!page)
> > - goto out;
> > + return -ENOMEM;
> >
> > if (!page_has_buffers(page))
> > create_empty_buffers(page, blocksize, 0);
> > @@ -2609,7 +2608,6 @@ int block_truncate_page(struct address_space *mapping,
> > pos += blocksize;
> > }
> >
> > - err = 0;
> > if (!buffer_mapped(bh)) {
> > WARN_ON(bh->b_size != blocksize);
> > err = get_block(inode, iblock, bh, 0);
> > @@ -2633,12 +2631,11 @@ int block_truncate_page(struct address_space *mapping,
> >
> > zero_user(page, offset, length);
> > mark_buffer_dirty(bh);
> > - err = 0;
>
> There is:
>
> if (!buffer_uptodate(bh) && !buffer_delay(bh) && !buffer_unwritten(bh))
> err = -EIO;
>
> above this assignment. So now we'll be returning -EIO if
> block_truncate_page() needs to read the block AFAICT. Did this pass fstests
> with some filesystem exercising this code (ext2 driver comes to mind)?
Hm, the code in current mainline is:
if (!buffer_uptodate(bh) && !buffer_delay(bh) && !buffer_unwritten(bh)) {
err = bh_read(bh, 0);
/* Uhhuh. Read error. Complain and punt. */
if (err < 0)
goto unlock;
}
Before e7ea1129afab ("fs/buffer: replace ll_rw_block()") that code really was
if (!buffer_uptodate(bh) && !buffer_delay(bh) && !buffer_unwritten(bh)) {
err = -EIO;
ll_rw_block(REQ_OP_READ, 1, &bh);
wait_on_buffer(bh);
/* Uhhuh. Read error. Complain and punt. */
if (!buffer_uptodate(bh))
goto unlock;
}
which would've indeed caused this change to return -EIO.
Is this still an issue now? Sorry if I'm being dense here.
On Mon 03-04-23 18:10:43, Jan Kara wrote:
> On Thu 23-03-23 10:32:59, Jiapeng Chong wrote:
> > Variable 'err' set but not used.
> >
> > fs/buffer.c:2613:2: warning: Value stored to 'err' is never read.
> >
> > Reported-by: Abaci Robot <[email protected]>
> > Link: https://bugzilla.openanolis.cn/show_bug.cgi?id=4589
> > Signed-off-by: Jiapeng Chong <[email protected]>
>
> I don't think the patch is quite correct (Christian, please drop it if I'm
> correct). See below:
Ah, sorry. I had too old kernel accidentally checked out. The patch is fine
with current upstream. Sorry for the noise!
Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR
On Mon, Apr 03, 2023 at 06:38:02PM +0200, Jan Kara wrote:
> On Mon 03-04-23 18:10:43, Jan Kara wrote:
> > On Thu 23-03-23 10:32:59, Jiapeng Chong wrote:
> > > Variable 'err' set but not used.
> > >
> > > fs/buffer.c:2613:2: warning: Value stored to 'err' is never read.
> > >
> > > Reported-by: Abaci Robot <[email protected]>
> > > Link: https://bugzilla.openanolis.cn/show_bug.cgi?id=4589
> > > Signed-off-by: Jiapeng Chong <[email protected]>
> >
> > I don't think the patch is quite correct (Christian, please drop it if I'm
> > correct). See below:
>
> Ah, sorry. I had too old kernel accidentally checked out. The patch is fine
> with current upstream. Sorry for the noise!
No problem at all. I'm very happy that you took the time to review this.
This is very much needed!
Christian
On Mon, Apr 03, 2023 at 05:13:19PM +0100, Matthew Wilcox wrote:
> On Mon, Mar 27, 2023 at 10:10:10AM +0200, Christian Brauner wrote:
> >
> > On Thu, 23 Mar 2023 10:32:59 +0800, Jiapeng Chong wrote:
> > > Variable 'err' set but not used.
> > >
> > > fs/buffer.c:2613:2: warning: Value stored to 'err' is never read.
> > >
> > >
> >
> > Applied to
>
> I think you should exercise extreme care with patches from "Abaci Robot".
> It's wrong more often than it's right, and the people interpreting the
> output from it do not appear to be experienced programmers.
Thank you! I've tried to be extra careful with these patches and will
continue to do so.