On Mon, Aug 23, 2021 at 08:47:35AM +0800, Yi Wang wrote: > From: Changcheng Deng <deng.changcheng@xxxxxxxxxx> > > A bug was found by coccinelle: > folio is NULL but dereferenced > Therefore,added a check to make sure 'folio' is not NULL. > > Reported-by: Zeal Robot <zealci@xxxxxxxxxx> Your robot is overzealous. This does not dereference folio; rather it takes the address of the page element of the folio structure. By a strict reading of the C spec, it is not allowed. However, GCC (and I assume Clang) does the right thing. > folio = __filemap_get_folio(mapping, index, fgp_flags, gfp); > - if ((fgp_flags & FGP_HEAD) || !folio || xa_is_value(folio)) > - return &folio->page; > + if ((fgp_flags & FGP_HEAD) || xa_is_value(folio)) > + if (folio != NULL) > + return &folio->page; > return folio_file_page(folio, index); This is definitely wrong. Did you test it? I bet you get a NULL pointer dereference if you try it. You could potentially make the case for: if (!folio) return NULL; if ((fgp_flags & FGP_HEAD) || xa_is_value(folio)) return &folio->page; but you actually have the same problem with the C spec, that unless folio is actually a pointer to a folio, then &folio->page is _technically_ undefined. So it would have to be something even more complex to be pedantically correct. It's just not worth it. Fix your tool.
