From owner-cvs-all Tue Sep 5 19:22:54 2000 Delivered-To: cvs-all@freebsd.org Received: from gatekeeper.tsc.tdk.com (gatekeeper.tsc.tdk.com [207.113.159.21]) by hub.freebsd.org (Postfix) with ESMTP id 6DFA537B422; Tue, 5 Sep 2000 19:22:49 -0700 (PDT) Received: from imap.gv.tsc.tdk.com (imap.gv.tsc.tdk.com [192.168.241.198]) by gatekeeper.tsc.tdk.com (8.8.8/8.8.8) with ESMTP id TAA23720; Tue, 5 Sep 2000 19:22:33 -0700 (PDT) (envelope-from gdonl@tsc.tdk.com) Received: from salsa.gv.tsc.tdk.com (salsa.gv.tsc.tdk.com [192.168.241.194]) by imap.gv.tsc.tdk.com (8.9.3/8.9.3) with ESMTP id TAA23635; Tue, 5 Sep 2000 19:22:33 -0700 (PDT) (envelope-from Don.Lewis@tsc.tdk.com) Received: (from gdonl@localhost) by salsa.gv.tsc.tdk.com (8.8.5/8.8.5) id TAA26628; Tue, 5 Sep 2000 19:22:32 -0700 (PDT) From: Don Lewis Message-Id: <200009060222.TAA26628@salsa.gv.tsc.tdk.com> Date: Tue, 5 Sep 2000 19:22:32 -0700 In-Reply-To: <39B581FE.7E78128B@netsynergy.co.uk> References: <200009052211.PAA70424@freefall.freebsd.org> <39B581FE.7E78128B@netsynergy.co.uk> X-Mailer: Mail User's Shell (7.2.6 beta(5) 10/07/98) To: Paul Richards , Don Lewis 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 Cc: cvs-committers@FreeBSD.org, cvs-all@FreeBSD.org Sender: owner-cvs-all@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG 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