Subject: Re: Adding KASSERT() calls to "sys/sys/queue.h"
To: None <tech-kern@netbsd.org>
From: Christos Zoulas <christos@zoulas.com>
List: tech-kern
Date: 06/17/2004 01:21:25
In article <20040616230954.GA1173@netbsd.org>,
Bill Studenmund <wrstuden@netbsd.org> wrote:
>-=-=-=-=-=-
>
>On Wed, Jun 16, 2004 at 08:48:11PM +0000, Matthias Scheler wrote:
>> 	Hello,
>> 
>> while I was examining PR kern/25868 I discoverd that a mbuf was freed twice
>> and therefore inserted into a pool twice. I finally found the code causing
>> the bug by modifying MFREE() like this:
>
>[snip]
>
>> They will of course not catch all possible errors but they would have
>> noticed the problem described above.
>> 
>> Opinions?
>
>I think it would be good to have such tests around. I'm not sure if they 
>should be turned on with all the other queue debugging or with a second, 
>additional control. I tend to think the latter, as exactly what error 
>we're looking for can vary from problem to problem; I thought the current 
>debug tests were to notice gross queue corruption whereas your check 
>catches incorrect queue use. But it could be that the test is cheep enough 
>that the difference doesn't matter.
>
>I am concerned though about the use of a bare KASSERT() though. It is
>reasonable to use sys/queue.h macros in userland, which doesn't have a
>KASSERT() defined (AFAICT). So a second define to turn on the KASSERT 
>would probably be best.

I think that we need to have sanity checks in the pooling code, controlled
with a define as opposed to ad hok pointers tests scattered all over the code.

christos