On Wed, Jun 14, 2023 at 04:01:12PM +0300, Dan Carpenter wrote:
> Hello Matthew Wilcox (Oracle),
>
> The patch dd69ce3382a2: "buffer: convert block_truncate_page() to use
> a folio" from Jun 12, 2023, leads to the following Smatch static
> checker warning:
>
> fs/buffer.c:1066 grow_dev_page() error: 'folio' dereferencing possible ERR_PTR()
>
> This one seems like a false positive, If you call __filemap_get_folio()
> with __GFP_NOFAIL then it only returns valid pointers, right?
That's right. There was no check for NULL before, and I looked at it
carefully before deciding that I didn't need to add a check for an error
when I converted it from find_or_create_page().
> fs/buffer.c:2689 block_truncate_page() warn: 'folio' is an error pointer or valid
> fs/buffer.c:2692 block_truncate_page() error: 'folio' dereferencing possible ERR_PTR()
>
> fs/buffer.c
> 2679 length = from & (blocksize - 1);
> 2680
> 2681 /* Block boundary? Nothing to do */
> 2682 if (!length)
> 2683 return 0;
> 2684
> 2685 length = blocksize - length;
> 2686 iblock = (sector_t)index << (PAGE_SHIFT - inode->i_blkbits);
> 2687
> 2688 folio = filemap_grab_folio(mapping, index);
> --> 2689 if (!folio)
>
> This should be IS_ERR().
Yup, that was sloppy.
Andrew, can you add this -fix to "buffer: Convert block_truncate_page()
to use a folio"?
diff --git a/fs/buffer.c b/fs/buffer.c
index 5a5b0c9d9769..248968dbde31 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -2617,8 +2617,8 @@ int block_truncate_page(struct address_space *mapping,
iblock = (sector_t)index << (PAGE_SHIFT - inode->i_blkbits);
folio = filemap_grab_folio(mapping, index);
- if (!folio)
- return -ENOMEM;
+ if (IS_ERR(folio))
+ return PTR_ERR(folio);
bh = folio_buffers(folio);
if (!bh) {
