svn commit: r330135 - head/sys/ufs/ffs

[Rodney W. Grimes]

> On Wed, Feb 28, 2018 at 5:59 PM, Rodney W. Grimes <
> freebsd at pdx.rh.cn85.dnsmgr.net> wrote:
> 
> > > Author: cem
> > > Date: Thu Mar  1 00:29:52 2018
> > > New Revision: 330135
> > > URL: https://svnweb.freebsd.org/changeset/base/330135
> > >
> > > Log:
> > >   ffs: softdep_disk_write_complete: Quiesce spurious Coverity warning
> > >
> > >   Coverity cannot determine that handle_written_indirdep() does not
> > access
> > >   uninitialized 'sbp' when flags argument is zero.
> > >
> > >   So, simply move the initialization slightly sooner to silence the
> > warning.
> > >
> > >   No functional change.
> >
> > I am always concerned when we modify code just to silence a
> > warning from a static analysis tool.  There use to be a pretty
> > hard and fast rule about this: Dont do it, at most document
> > that the tool is lacking and giving false positives.
> >
> > It double raises an eye brow when the change is made to
> > silence the tool and "no function change" should be occuring.
> 
> 
> In this case, there's no harm, and I think it makes the code a little
> easier to understand...

If infact handle_written_indirdep() should not access sbp then it
would be safe to pass it a void * and let the code blow up if it
should derefernce it.  Thus simply changing the call that was
causing the problem and not moving any lines of code.

> Warner
> 
> 
> > >   Reported by:        Coverity
> > >   Sponsored by:       Dell EMC Isilon
> > >
> > > Modified:
> > >   head/sys/ufs/ffs/ffs_softdep.c
> > >
> > > Modified: head/sys/ufs/ffs/ffs_softdep.c
> > > ============================================================
> > ==================
> > > --- head/sys/ufs/ffs/ffs_softdep.c    Thu Mar  1 00:05:14 2018
> > (r330134)
> > > +++ head/sys/ufs/ffs/ffs_softdep.c    Thu Mar  1 00:29:52 2018
> > (r330135)
> > > @@ -10984,6 +10984,8 @@ softdep_disk_write_complete(bp)
> > >       if (ump == NULL)
> > >               return;
> > >
> > > +     sbp = NULL;
> > > +
> > >       /*
> > >        * If an error occurred while doing the write, then the data
> > >        * has not hit the disk and the dependencies cannot be processed.
> > > @@ -11026,7 +11028,6 @@ softdep_disk_write_complete(bp)
> > >       /*
> > >        * Ump SU lock must not be released anywhere in this code segment.
> > >        */
> > > -     sbp = NULL;
> > >       owk = NULL;
> > >       while ((wk = LIST_FIRST(&bp->b_dep)) != NULL) {
> > >               WORKLIST_REMOVE(wk);
> > >
> > >
> >
> > --
> > Rod Grimes
> > rgrimes at freebsd.org
> >
> >

-- 
Rod Grimes                                                 rgrimes at freebsd.org