svn commit: r328430 - head/sbin/devd

Ian Lepore ian at freebsd.org
Fri Jan 26 16:56:09 UTC 2018


On Thu, 2018-01-25 at 22:59 -0700, Warner Losh wrote:
> On Thu, Jan 25, 2018 at 10:02 PM, Matt Joras <matt.joras at gmail.com> wrote:
> 
> > 
> > On Thu, Jan 25, 2018 at 8:52 PM, Warner Losh <imp at bsdimp.com> wrote:
> > > 
> > > 
> > > 
> > > On Thu, Jan 25, 2018 at 9:40 PM, Eitan Adler <eadler at freebsd.org> wrote:
> > > > 
> > > > 
> > > > Author: eadler
> > > > Date: Fri Jan 26 04:40:41 2018
> > > > New Revision: 328430
> > > > URL: https://svnweb.freebsd.org/changeset/base/328430
> > > > 
> > > > Log:
> > > >   devd: minor nits
> > > > 
> > > >   - mark usage as noreturn
> > > >   - config does not need a virtual destructor
> > > 
> > > Everything needs a virtual destructor...  Please back that part of this
> > > out...
> > > 
> > > Warner
> > Needs? If there's not inheritance there's no _need_ to do it, and it's
> > arguably superfluous. That being said it's also an arguably
> > superfluous change to remove it.
> > 
> OK. Needs might not be the proper word. It's a stylistic thing in all the
> C++ code I write. It keeps me from having to go back later and add it back
> when it really does need it after subclasses are derived from it. It's an
> important detail that often gets overlooked. The cost is minimal. And this
> isn't one of the rare cases where it's harmful to have it.
> 
> Warner

Bah.  When a class is not designed for inheritence, it should NOT have
a virtual dtor.  Not just because of the extra overhead of vtable
dispatching where it isn't needed, but also because it lulls someone
who comes along and looks at the class into thinking it was designed to
be derived-from.  If it wasn't specifically designed to be derived-
from, then before doing so somebody needs to make sure the class is
ready for that.

Modern compilers will warn about a class with virtual functions and no
virtual dtor, so just blindly including it is more harmful than
prophylactic these days, IMO.

-- Ian



More information about the svn-src-head mailing list