On Tue, Aug 27, 2019 at 12:53:47PM +0300, Dan Carpenter wrote:
> On Tue, Aug 27, 2019 at 05:36:29PM +0800, Gao Xiang wrote:
> > Hi Dan,
> >
> > Thanks for your report.
> >
> > On Tue, Aug 27, 2019 at 12:03:55PM +0300, Dan Carpenter wrote:
> > > Hello Gao Xiang,
> > >
> > > This is a semi-automatic email about new static checker warnings.
> > >
> > > The patch 97e86a858bc3: "staging: erofs: tidy up decompression
> > > frontend" from Jul 31, 2019, leads to the following Smatch complaint:
> > >
> > > fs/erofs/zdata.c:670 z_erofs_do_read_page()
> > > error: we previously assumed 'clt->cl' could be null (see line 596)
> > >
> > > fs/erofs/zdata.c
> > > 595 /* didn't get a valid collection previously
> > > (very rare) */
> > > 596 if (!clt->cl)
> > > ^^^^^^^^
> > > New NULL check.
> > >
> > > 597 goto restart_now;
> > > 598 goto hitted;
> > > 599 }
> > > 600
> > > 601 /* go ahead the next map_blocks */
> > > 602 debugln("%s: [out-of-range] pos %llu", __func__, offset
> > > + cur);
> > > 603
> > > 604 if (z_erofs_collector_end(clt))
> > > 605 fe->backmost = false;
> > > 606
> > > 607 map->m_la = offset + cur;
> > > 608 map->m_llen = 0;
> > > 609 err = z_erofs_map_blocks_iter(inode, map, 0);
> > > 610 if (unlikely(err))
> > > 611 goto err_out;
> > > 612
> > > 613 restart_now:
> > > 614 if (unlikely(!(map->m_flags & EROFS_MAP_MAPPED)))
> > > 615 goto hitted;
> > > 616
> > > 617 err = z_erofs_collector_begin(clt, inode, map);
> >
> > At a glance, clt->cl will be all initialized in all successful paths
> > in z_erofs_collector_begin, or it all fall back into err_out...
> > I have no idea what is wrong here...
> >
> > Some detailed path from Smatch for NIL dereferences?
> >
>
> Ah. Sorry for that. It's a false positive. I will investigate and
> fix Smatch.
Yeah.. I was little confused, since this patch mostly renames many names...
and the main logic is unchanged for months... and for this case there are 2
paths...
1) hit line 614 --> goto hitted --> hit line 633 --> goto next_part; (will
skip line 670);
2) hit line 617 --> go into z_erofs_collector_begin -->
all successful paths will assign clt->cl, so clt->cl != NULL...
Though z_erofs_do_read_page is currently somewhat complicated (mostly due to
some
historical fixes in order to backport friendly), I will simplify this function
in the later version (..and with care in case of introducing new bugs) :-)
Thanks,
Gao Xiang
>
> regards,
> dan carpenter
>
