Re: [linux-next:master 7893/8235] fs/jfs/jfs_metapage.c:245 __metapage_migrate_folio() error: we previously assumed 'mp' could be null (see line 235)

[Dan Carpenter <dan.carpenter@linaro.org>]

On Wed, May 07, 2025 at 05:19:52PM +0530, Shivank Garg wrote:
> 
> Hi Dan,
> 
> This is a false positive.
> 
> In metapage_migrate_folio(), it checks if (!src->private) and only calls
> __metapage_migrate_folio() if src->private is non-NULL
> 
> static int metapage_migrate_folio(struct address_space *mapping, struct folio *dst,
> 				  struct folio *src, enum migrate_mode mode)
> {
> ...
> 	if (!src->private)
> 		return filemap_migrate_folio(mapping, dst, src, mode);
> ...
> ...
> 	return __metapage_migrate_folio(mapping, dst, src, mode);
> }
> 
> Then __metapage_migrate_folio() calls mp = folio_to_mp(src, 0), which returns src->private.
> Which should not be NULL as previously checked.
> 
> I think we can skip the mp != NULL check from 
> > 39ed4d1a0e03ce Shivank Garg  2025-04-30 @235  	if (mp && metapage_locked(mp))
> It seem redundant here.

Yep.  Just delete the unnecessary NULL check.

I only consider these a false positive if the NULL check is
required but it doesn't lead to a crash.  For example, there could be
another condition which is equivalent to checking for NULL.

	if (p)
		p->foo = 100;
	...
	if (!p_valid(p))
		return;

	p->foo = 99;

In this code, if you have the cross function database then Smatch
doesn't print a warning because the NULL check is not required.  But
the cross function database is way too slow to use in the kbuild-bot.

regards,
dan carpenter