lkml.org 
[lkml]   [1999]   [Jun]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH] *(int*)0 = 0 & variations
Hi Jeff.

> 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.

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.

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.

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.

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.

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:

===8<=== CUT ===>8===
--- linux/include/linux/kassert.h~ Thu Jan 1 01:00:00 1970
+++ linux/include/linux/kassert.h Thu Jun 24 01:02:30 1999
@@ -0,0 +1,20 @@
+/*
+ * Include definitions for kernel assertion checking
+ */
+
+#ifndef __KASSERT_H__
+#define __KASSERT_H__
+
+#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")
+#else
+#define kassert(cond) (void) abs(cond)
+#endif
+
+#endif
===8<=== CUT ===>8===
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/

\
 
 \ /
  Last update: 2005-03-22 13:52    [W:0.060 / U:0.152 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site