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

[Warner Losh]

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...

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
>
>