Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 5 Sep 2000 19:22:32 -0700
From:      Don Lewis <Don.Lewis@tsc.tdk.com>
To:        Paul Richards <paul@netsynergy.co.uk>, Don Lewis <truckman@FreeBSD.org>
Cc:        cvs-committers@FreeBSD.org, cvs-all@FreeBSD.org
Subject:   Re: cvs commit: src/sys/kern init_main.c kern_exec.c kern_exit.ckern_fork.c kern_proc.c kern_prot.c kern_resource.c uipc_socket.cuipc_socket2.c uipc_usrreq.c vfs_aio.c src/sys/sys proc.hresourcevar.h ucred.h
Message-ID:  <200009060222.TAA26628@salsa.gv.tsc.tdk.com>
In-Reply-To: <39B581FE.7E78128B@netsynergy.co.uk>
References:  <200009052211.PAA70424@freefall.freebsd.org> <39B581FE.7E78128B@netsynergy.co.uk>

next in thread | previous in thread | raw e-mail | index | archive | help
On Sep 6, 12:30am, Paul Richards wrote:
} Subject: Re: cvs commit: src/sys/kern init_main.c kern_exec.c kern_exit.ck
} Don Lewis wrote:
} > 
} 
} >   Change KASSERTs in this code to unconditional tests and calls to panic().
} 
} I think we should have some style guidelines about KASSERTS, INVARIANTS
} and panic et al.

I agree with this.  I think we also need to be able to more easily tune
these.  Currently quite a bit of editing is needed to switch between
	if (...)
		panic();
and
	KASSERT(...);

} I don't think panic should be used unless it is an environmental effect
} that the kernel needs to trap i.e. a disk is failing and it's better to
} panic if data looks funny than make corruption worse, for example.

The previous implementation didn't do enough sanity checking, and
much of the checking that it did was done as KASSERTs.  The lack of
manditory checking was letting problems slip through that could later
cause panics.

} A panic shouldn't be used to catch bugs, those should be tested for
} using debugging tools such as KASSERTS wrapped in INVARIANTS, even if
} they then call panic to get a core dump.
} 
} (I haven't looked at these diffs and there may be good security reasons
} for always checking and calling panic in this case but in general
} there's been a trend recently to put debugging code in the production
} code rather than putting it inside INVARIANTS.)

After some prodding by bde who said that any sanity checks in new code
should be mandatory, I changed this before I did my commit.  In this
case I may have erred in the other direction, since my new implementation
does things more safely than the previous implementation.

Maybe the -current version of GENERIC should enable INVARIANTS, for
the same reason that we crank up malloc debugging.  We should probably
also have tuneable levels of sanity checking and have an easy way to
tune the level at which each sanity check is activated without requiring
a lot of change to the source.


To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe cvs-all" in the body of the message




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