Messages in this thread Patch in this message | | | Date | Thu, 24 Jun 1999 01:13:59 +0100 (GMT) | From | Riley Williams <> | Subject | Re: [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/
| |