Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 19 Jul 2006 11:38:40 +0400
From:      Yar Tikhiy <yar@comp.chem.msu.su>
To:        Bruce Evans <bde@zeta.org.au>
Cc:        cvs-src@FreeBSD.org, src-committers@FreeBSD.org, Giorgos Keramidas <keramida@FreeBSD.org>, cvs-all@FreeBSD.org
Subject:   Re: cvs commit: src/sys/net if.c if_atmsubr.c if_stf.c if_tun.c src/sys/netinet if_ether.c ip_divert.c ip_fw2.c src/sys/netinet6 in6.c in6_var.h src/sys/nfsclient bootp_subr.c nfs_diskless.c
Message-ID:  <20060719073840.GC69362@comp.chem.msu.su>
In-Reply-To: <20060719131841.U41630@delplex.bde.org>
References:  <200606291922.k5TJM5ev007314@repoman.freebsd.org> <20060701003326.GA41947@gothmog.pc> <20060719131841.U41630@delplex.bde.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, Jul 19, 2006 at 01:32:26PM +1000, Bruce Evans wrote:
> Long ago, On Sat, 1 Jul 2006, Giorgos Keramidas wrote:
> 
> >On 2006-06-29 19:22, Yar Tikhiy <yar@freebsd.org> wrote:
> >>yar         2006-06-29 19:22:05 UTC
> >>
> >>  FreeBSD src repository
> >>
> >>  Modified files:
> >>    sys/net              if.c if_atmsubr.c if_stf.c if_tun.c
> >>    sys/netinet          if_ether.c ip_divert.c ip_fw2.c
> >>    sys/netinet6         in6.c in6_var.h
> >>    sys/nfsclient        bootp_subr.c nfs_diskless.c
> >>  Log:
> >>  There is a consensus that ifaddr.ifa_addr should never be NULL,
> >>  except in places dealing with ifaddr creation or destruction; and
> >>  in such special places incomplete ifaddrs should never be linked
> >>  to system-wide data structures.  Therefore we can eliminate all the
> >>  superfluous checks for "ifa->ifa_addr != NULL" and get ready
> >>  to the system crashing honestly instead of masking possible bugs.
> >
> >This is probably silly, but it was the first thing I thought about when
> >I saw the NULL checks removed.
> >
> >Since we assume that ifa->ifa_addr != NULL, does it make sense to add
> >KASSERT() calls in the places where we do so?
> 
> No, that would be worse than leaving the checks unchanged.  Asserting
> that pointers aren't null just re-bloats the code (at least at the
> source level) and breaks normal handling of dereferencing of null
> pointers.  With normal handling, you get a trap that can be restarted
> using a debugger, but with assertions (if assertions are enabled) you
> get a panic that can't be restarted (modulo the RESTARTABLE_PANICS
> option which causes other problems).

Seconded.  I believe that KASSERT() shouldn't be routinely used for
NULL checks, it's for more complex consistency tests.

-- 
Yar



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