Re: [PATCH] *(int*)0 = 0 & variations

Riley Williams (rhw@MemAlpha.CX)
Fri, 25 Jun 1999 00:47:18 +0100 (GMT)


Hi Jeff.

>>>> +#else
>>>> +#define kassert(cond) (void) abs(cond)
>>>> +#define kassertoops(cond) (void) abs(cond)
>>>> +#endif

>>> Any code depending on assert evaluating the condition is broken
>>> IMHO.

>> IMHO also, but the general concensus appears to be in favour
>> of it. I've put an '#if 1'-#else-#endif block in the code,
>> defaulting to not evaluating, but changing the '#if 1' line
>> to '#if 0' inverts that.

> What consensus do you see here? Two people?

The figures so far are that I received three responses in Linux kernel
saying that for kassert not to evaluate its parameter would be a big
cause of bugs, and four further private responses making the same
point, against just your response saying otherwise. That's 7 to 1 in
favour of the evaluation - or 7 to 2 if one includes the fact that my
personal belief is that it shouldn't.

> I didn't even see any cases where this behavior was said to be
> used -- only said to be needed in a few rare cases.

> AC posted in this thread, saying that assert code already exists
> in the kernel, in the networking code (grep for BUG_TRAP).
> Guess what? It evaluates to null. In fact, grepping around,
> every assert macro I could find in the kernel (not many,
> granted) evaluates to null in its non-debug form.

> Evaluating to null is the standard set by both the existing
> kernel sources and ANSI C. NOT evaluating to null potentially
> slows down a fast path, if a kassert() is used there. Why use
> it, if it breaks with tradition and creates slower production
> code?

That's my opinion as well, which is why I had the undefined case not
evaluate it as submitted.

> So, my suggested updates to your kassert.h:

> o Evaluate to null in non-debug form ;-)

Done.

> Copy code from BUG_TRAP instead of simply "#define
> kassert(cond)". It wraps a null do-while.

Wilco.

> o Allow developer to redefine REPORT_LEVEL by wrapping it in an
> #ifndef

I'm not quite sure what you're referring to here...

1. Allow different developers to set different REPORT_LEVEL values,
such that (for example) the kassert() lines in the ISDN subsystem
report failures as KERN_CRIT but those in the sound subsystem
report failures as KERN_EMERG ???

If this is what you intended to imply, then I would have to state
that I consider such to be a BAD idea, and would need persuading
that such was a reasonable course of action before I would begin
to consider doing so.

2. Allow a developer to specify the REPORT_LEVEL to be used across
the kernel ???

The current kernel allows this, since the setting of REPORT_LEVEL
in kassert.h is used throughout.

3. Some other meaning?

Please clarify.

> o Is there some central file that the CONFIG_xxx selection can
> be moved to, so avoid changing each port's Config.in? Maybe
> source linux/kernel/Config.in (new file) from each port instead.

> There are too much common stuff in the port Config.in's
> anyway, IMHO.

Personally, I'd like to see the configuration system redesigned such
that the main config.in file was in the main Linux directory, and it
included the relevant arch/*/*/config.in files in the relevant
sections of the main ones if they existed for that port. This would
result in something like the following syntax being added to ALL of
the configuration scripts:

if [ -f $BASE/arch/i386/kernel/Config.in ]; then
source $BASE/arch/i386/kernel/Config.in
fi

...where the -f says "what follows exists and is either a regular file
or a symlink that resolves to a regular file", as per the bash script
standard, and $BASE expands to the absolute path of the base Linux
directory, probably /usr/src/linux on most systems.

Actually, I proposed a variant of that a while back which integrated
the configuration and help system together to ensure consistancy. This
variant would result in separate config.in and Configure.help files in
each directory containing options specific to that directory, and all
the responses were of the "go for it" variety.

The problem is that it was just a couple of days before my first exam
when I proposed it, and as a result, I never had time to look into it
myself. Also, ALL of the *config scripts would need to be updated in
parallel to do this, and, whilst I can probably handle the changes to
both configure and menuconfig, I couldn't even begin to sort out
xconfig at all...

Best wishes from Riley.

+----------------------------------------------------------------------+
| There is something frustrating about the quality and speed of Linux |
| development, ie., the quality is too high and the speed is too high, |
| in other words, I can implement this XXXX feature, but I bet someone |
| else has already done so and is just about to release their patch. |
+----------------------------------------------------------------------+
* ftp://ftp.MemAlpha.cx/pub/rhw/Linux
* http://www.MemAlpha.cx/kernel.versions.html

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/