Date: Sat, 9 Nov 1996 00:09:44 +0900 (JST) From: Michael Hancock <michaelh@cet.co.jp> To: David Greenman <dg@root.com> Cc: freebsd-hackers@freefall.freebsd.org Subject: Re: More info on the daily panics... Message-ID: <Pine.SV4.3.95.961108232542.5436A-100000@parkplace.cet.co.jp> In-Reply-To: <199611081024.CAA05116@root.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, 8 Nov 1996, David Greenman wrote:
> I don't think you understand: I understand your position on this and the
> benefits as you've articulated them are very clear. I simply disagree with
> you on this topic. My opinion is that I don't like what it does to the sources
> (adding lots of redundant and usually useless assertions), nor do I like the
Actually, this is where you misunderstand.
I am not an advocate of defensive programming. Defensive programming
means that you add a lot of redundant checks and you write code assuming
the unexpected. This is bad because it becomes hard to find the real
code from all the checking code.
I see the use of assertions as a way of making it so that we can have
expections of how things operate. The writer of the function specifies
his expectations and the consumer of the function doesn't have to write
redundant checks before calling the function because he knows what to
expect. The writer of the function also doesn't have to do redundant
checking when passing the parameters to another function. And maybe the
the latter function doesn't need to do checking because it is a helper
function and it can assume that it's consumer has already done the
checking.
If assertions are applied properly they can actually reduce the number of
checks done in the code and this is what you are missing completely.
You can't tell me that there aren't redundant and unnecessary checks in
the fs code already. I advocate that some of these are actually taken
out and others moved to where they are "strategic".
If you can't swallow the macros then please consider the following:
Proactive use of FreeBSD assertion model.
====================================================================
void
vrele(vp)
register struct vnode *vp;
{
/* parameter checking section */
#ifdef DIAGNOSTIC
if (vp == NULL)
panic("vrele: null vp");
#endif
if (vp->v_usecount <= 0) {
#ifdef DIAGNOSTIC
vprint("vrele: negative ref count", vp);
#endif
panic("vrele: negative reference cnt");
}
/* start of actual code body */
vp->v_usecount--;
if ((vp->v_usecount == 1) &&
vp->v_object &&
(vp->v_object->flags & OBJ_VFS_REF)) {
vp->v_object->flags &= ~OBJ_VFS_REF;
vm_object_deallocate(vp->v_object);
return;
}
if (vp->v_usecount > 0)
return;
[ .. ]
Reactive use of FreeBSD assertion model.
====================================================================
void
vrele(vp)
register struct vnode *vp;
{
#ifdef DIAGNOSTIC
if (vp == NULL)
panic("vrele: null vp");
#endif
vp->v_usecount--;
if ((vp->v_usecount == 1) &&
vp->v_object &&
(vp->v_object->flags & OBJ_VFS_REF)) {
vp->v_object->flags &= ~OBJ_VFS_REF;
vm_object_deallocate(vp->v_object);
return;
}
if (vp->v_usecount > 0)
return;
if (vp->v_usecount < 0) {
#ifdef DIAGNOSTIC
vprint("vrele: negative ref count", vp);
#endif
panic("vrele: negative reference cnt");
}
[ ... ]
============================================================
The changes are subtle but the improvement is that we have a body of code
that only does real work and we have a body of code that is only debugging
infrastructure.
Instead of doing an operation and seeing if we have garbage, we refuse to
accept garbage period.
The distinction is important.
Regards,
Mike Hancock
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.SV4.3.95.961108232542.5436A-100000>