From owner-freebsd-hackers  Fri Nov  8 07:09:59 1996
Return-Path: owner-hackers
Received: (from root@localhost)
          by freefall.freebsd.org (8.7.5/8.7.3) id HAA08996
          for hackers-outgoing; Fri, 8 Nov 1996 07:09:59 -0800 (PST)
Received: from parkplace.cet.co.jp (parkplace.cet.co.jp [202.32.64.1])
          by freefall.freebsd.org (8.7.5/8.7.3) with ESMTP id HAA08986
          for <freebsd-hackers@freefall.freebsd.org>; Fri, 8 Nov 1996 07:09:55 -0800 (PST)
Received: from localhost (michaelh@localhost) by parkplace.cet.co.jp (8.8.2/CET-v2.1) with SMTP id PAA05711; Fri, 8 Nov 1996 15:09:44 GMT
Date: Sat, 9 Nov 1996 00:09:44 +0900 (JST)
From: Michael Hancock <michaelh@cet.co.jp>
Reply-To: 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... 
In-Reply-To: <199611081024.CAA05116@root.com>
Message-ID: <Pine.SV4.3.95.961108232542.5436A-100000@parkplace.cet.co.jp>
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII
Sender: owner-hackers@FreeBSD.ORG
X-Loop: FreeBSD.org
Precedence: bulk

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