Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 28 Feb 2018 18:21:41 -0700
From:      Warner Losh <imp@bsdimp.com>
To:        "Rodney W. Grimes" <rgrimes@freebsd.org>
Cc:        Conrad Meyer <cem@freebsd.org>, src-committers <src-committers@freebsd.org>, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   Re: svn commit: r330135 - head/sys/ufs/ffs
Message-ID:  <CANCZdfqizVa4ZNOT2DH8N3NmqBFsgrV65QVcRBpi-R-irZbY1w@mail.gmail.com>
In-Reply-To: <201803010059.w210xoZ3022625@pdx.rh.CN85.dnsmgr.net>
References:  <201803010029.w210TqF5069963@repo.freebsd.org> <201803010059.w210xoZ3022625@pdx.rh.CN85.dnsmgr.net>

next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, Feb 28, 2018 at 5:59 PM, Rodney W. Grimes <
freebsd@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@freebsd.org
>
>



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CANCZdfqizVa4ZNOT2DH8N3NmqBFsgrV65QVcRBpi-R-irZbY1w>