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

[Giorgos Keramidas]

On 2006-07-19 13:32, Bruce Evans <bde at zeta.org.au> wrote:
>Long ago, On Sat, 1 Jul 2006, Giorgos Keramidas wrote:
>>On 2006-06-29 19:22, Yar Tikhiy <yar at 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).

Subtle, but important difference.  Thanks for taking the time to write
down the explanation :-)