Christos Zoulas <christos@netbsd.org> writes:

> Module Name:	src
> Committed By:	christos
> Date:		Fri Apr 14 17:07:23 UTC 2006
>
> Modified Files:
> 	src/sys/dev/usb: ohci.c
>
> Log Message:
> Coverity CID 1115: It is quite pointless to have a DIAGNOSTIC panic that
> checks a variable for being NULL, and if we are not in DIAGNOSTIC code, to
> just dereference it causing a crash!
>
> To generate a diff of this commit:
> cvs rdiff -r1.171 -r1.172 src/sys/dev/usb/ohci.c

I don't follow your comment or your fix.  I have always believed (but
could well be off) that DIAGNOSTIC code should verify invariants that
must always be true, but that given a non-zero defect density might
not be.  Thus, DIAGNOSTIC is an explicit tradeoff of executing more
instructions and being more likely to catch an "impossible" condition
rather than e.g. dereferencing a NULL pointer.

The old code followed my understanding of DIAGNOSTIC.  Now, if the
pointer is NULL, the variables aren't updated, but the function
continues if DIAGNOSTIC isn't true.  Thus an invariant violation is
turned into what looks like a failure to follow the function
specification.

Do you mean that in this case it's ok not to find the entry?
But there's still a panic in DIAGNOSTIC, so it seems not.

KASSERT does essentially the same thing as #ifdef DIAGNOSTIC/panic,
and it seems to be used to "fix" a number of these problems.
Why did you make the change you did rather than converting the ifdef
DIAGNOSTIC/panic to a KASSERT(p != NULL)?

Thanks for all the fixes, and for putting in descriptive CVS commit
messages.   It will be very cool if all these end up fixing the ohci
cardbus detach crash.

-- 
        Greg Troxel <gdt@ir.bbn.com>