On Wed, 23 Jun 1999, Jeff Garzik wrote:
>>> assert should definitely be more informative, and an assertion
>>> failure is more than a debug statement. The following functions
>>> more like the userland version:
>>> #ifndef NDEBUG
>>> # define kassert(cond) \
>>> if (!(cond)) { \
>>> printk (KERN_ERR "kassert failed in function %s, file %s, line %d:
>>> %s\n", \
>>> __FUNCTION__, \
>>> __FILE__, \
>>> __LINE__, \
>>> __STRING(cond)); \
>>> *(int*)0 = 0; \
>>> }
>>> #else
>>> # define kassert(cond)
>>> #endif
>>> You could put some additional switches in there to enable or
>>> disable the oops after the printk, or to change KERN_ERR to
>>> something else.
>> I would suggest you think about the context in which kassert
>> would be used before implementing the above in the kernel.
>> Perhaps I can point out some relevant issues that you appear
>> to have overlooked:
>> 1. Whatever the assertion that failed, the kernel NEEDS to keep
>> running. Therefore, an OOPS is usually NOT appropriate.
> Personally I disagree, but I do agree that for the general
> population you are right.
One thing I would HATE to see is Linux shooting itself in the foot by
having a kassert somewhere that causes some system to become unusable.
That's the main reason I'm against the idea of kassert auto-oops'ing.
> How about a switch in kassert.h that optionally causes an oops?
> If you wanted to get really fancy, take that print-backtrace
> code that someone posted in this thread, and use that before
> _optionally_ killing the kernel.
I'd be far more inclined to define two separate functions, kassert()
which prints the message and then continues as if it's not there, and
kassertoops() which prints the message and oops. That way, the choice
of function would document which was desired, and the developer could
use whichever was appropriate.
There is also the fact that by its nature, assertion checking makes
most of the standard oops report redundant as, if kassertoops was to
be used, the exact circumstances would be known in advance. It would
therefore make more sense to have two separate oops functions, one to
deal with the current oops events, and a second to deal with an oops
caused by a kassertoops() call.
>> 2. You state that an "assertion failure is more than a debug
>> statement" without providing any explanation for your claim.
>> As I see it, and taking kassert in its purest way, its ONLY
>> purpose is debugging, and it is thus ideal for KERN_DEBUG.
> If an assertion fails something is REALLY WRONG, and should be
> printed out as a kernel message. The most typical use I've seen
> for assertions is to warn if a null pointer if about to be
> dereferenced, and you definitely want to make that known. loudly.
I have to admit that such is NOT a use to which I have ever put
assert() and to my way of thinking, it's the wrong way of dealing with
it.
> Assertions should NOT occur, even in debug kernels. An assertion
> is not a FIXME. Failure of an assertion means the impossible
> just occured.
To be accurate, the failure of an assertion indicates that the person
who developed that particular piece of code made an assumption that
turned out not to be valid, and the purpose of the resultant message
is to point out that fact. It is in this context that any assertion
reports are most valuable, and this context is ONLY relevant when the
user is willing to do something with the said information.
Many years of experience has taught me that what I call the "Microsoft
Rule of Thumb" applies to most users - if an error message appears on
the screen, reformat the hard drive and reinstall everything. One
thing Linux is far better than M$ at is recovering from seemingly
impossible errors, and Linux is getting a reputation for that as well.
Misimplement something like kassert() and that reputation will vanish,
and for all the wrong reasons as well.
>> 3. As far as actually seeing the error reports are concerned,
>> the majority of users would have NO idea what to do if some
>> application started spewing "assertion failure" messages all
>> over their screen, so using a reporting level that is often
>> set up to spew all suchlike messages to all logged in users
>> is not a particularly good idea.
> They will report it as a bug report like a good little user.
Since when !!!
>> 4. The way that I would see such a facility used is for users
>> with enough interest to help with the kernel to set up their
>> system such that KERN_DEBUG errors get logged to a specific
>> file, which would then serve as a central repository for ALL
>> kernel debugging messages.
>> The default setting, used by the majority of the users (who have
>> no interest in debugging Linux) being that suchlike messages are
>> logged in either /var/log/messages or in /dev/null as preferred.
> I disagree, see above. Assertion failures are very important.
Important, yes - but ONLY to people interested in kernel development,
and even then only to document assumptions the developer has made. If
they're used as an error detecting system, as you appear to be
proposing, then they're being seriously misused.
>> 5. The choice of '#ifndef NDEBUG' or '#ifdef DEBUG' matters, as the
>> two forms have different implications. Personally, I believe the
>> latter has the correct implication in the kernel context, but the
>> former has the correct implication in an application context:
>> '#ifndef NDEBUG' sayd "Disable debugging only if the variable
>> NDEBUG is defined", thus implying that the normal case is WITH
>> debugging.
>> '#ifdef DEBUG' says "Enable debugging only if the variable DEBUG
>> is defined", thus implying that the normal case will be without
>> debugging.
> I know.
> I, and the C designers, used '#ifndef NDEBUG' on purpose.
Like I said, in an APPLICATION context, '#ifndef NDEBUG' is normally
correct, but I do not believe it is also correct in a KERNEL context.
>> 6. Whilst the format of the userland version is interesting, it is
>> not necessarily a useful guide to the format the kernel version
>> should take. Remember that both the circumstances of its use and
>> the results thereof are considerably different.
>>
>> For these reasons, and taking into account the various comments made
>> so far, I would suggest the following patch instead:
> Does this print out the expression correctly?? I would think
> that this is more correct:
> #define kassert(cond)
> if (!cond)
> printf("assertion '" #cond "' failed\n");
I've just checked, and you are of course correct: My version doesn't
print out the condition correctly with gcc. I'm far too used to
Borland's TurboC for which my version worked - I just copied it from a
program I wrote and had to hand, where the standard assert() behaviour
of calling abort() was inappropriate.
Here's the corrected version:
--- linux/include/linux/kassert.h~ Thu Jan 1 01:00:00 1970
+++ linux/include/linux/kassert.h Thu Jun 24 02:19:10 1999
@@ -0,0 +1,41 @@
+/*
+ * Include definitions for kernel assertion checking
+ */
+
+#ifndef __KASSERT_H__
+#define __KASSERT_H__
+
+/*
+ * Generate an oops
+ */
+
+#define oops() *(int *)0 = 0
+
+/*
+ * Define the kassert() and kassertoops() macros appropriately.
+ */
+
+#ifdef DEBUG
+#define kassert(cond) \
+ if (cond) { \
+ /* Do nothing */; \
+ } else { \
+ printk( KERN_DEBUG "ASSERTION FAILURE: %s line %u: %s\n" \
+ "Assertion = (%s)\n", \
+ __FILE__, __LINE__, __FUNCTION__, #cond ); \
+ }
+#define kassertoops(cond) \
+ if (cond) { \
+ /* Do nothing */; \
+ } else { \
+ printk( KERN_DEBUG "ASSERTION FAILURE: %s line %u: %s\n" \
+ "Assertion = (%s)\n", \
+ __FILE__, __LINE__, __FUNCTION__, #cond ); \
+ oops(); \
+ }
+#else
+#define kassert(cond) (void) abs(cond)
+#define kassertoops(cond) (void) abs(cond)
+#endif
+
+#endif
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/