Re: KASSERTMSG fix

[Jean-Yves Migeon <jeanyves.migeon%free.fr@localhost>]

On 17.09.2011 01:09, Jean-Yves Migeon wrote:
> [snip]
> > Summary about my discussion regarding KASSERTMSG(): in its current form
> > it cannot append the msg passed as argument to it to the panic format
> > string, which makes it rather impractical: we fail to log the file,
> > line, expression that triggered the assertion and worse, it does not
> > conform to its description in KASSERT(9).
  >[snip]
> > So I am back with the __VA_ARGS__ tricks; with the attached patch and
> > all the KASSERT call sites fixed, the ALL kernel size is actually
> > smaller... [by about 20k]
>
> I am attaching the patch I wrote for the proposal above (not attaching
> the patch for call sites, it's worthless for now)
>
> Comments are welcomed, I'd like to improve the KASSERTMSG(...) macro a
> bit. Currently, it's just a mere alias for an if + panic(9).

Unless someone objects (with arguments), I will commit this at the end 
of the week and fix call sites to use the new macro; KASSERTMSG() would 
then work like its man page suggests.

Thanks!

--
Jean-Yves Migeon
jeanyves.migeon%free.fr@localhost

Index: sys/lib/libkern/libkern.h
===================================================================
RCS file: /cvsroot/src/sys/lib/libkern/libkern.h,v
retrieving revision 1.99
diff -u -p -r1.99 libkern.h
--- sys/lib/libkern/libkern.h   1 Sep 2011 22:35:17 -0000       1.99
+++ sys/lib/libkern/libkern.h   25 Sep 2011 13:57:49 -0000
@@ -37,6 +37,7 @@
 #include <sys/types.h>
 #include <sys/inttypes.h>
 #include <sys/null.h>
+#include <sys/systm.h>
 
 #ifndef LIBKERN_INLINE
 #define LIBKERN_INLINE static __inline
@@ -174,15 +175,17 @@ tolower(int ch)
 
 #define        __NULL_STMT             do { } while (/* CONSTCOND */ 0)
 
+#define __KASSERTSTR  "kernel %sassertion \"%s\" failed: file \"%s\", line %d "
+
 #ifdef NDEBUG                                          /* tradition! */
 #define        assert(e)       ((void)0)
 #else
 #ifdef __STDC__
 #define        assert(e)       (__predict_true((e)) ? (void)0 :                
    \
-                           kern_assert("", __FILE__, __LINE__, #e))
+                           panic(__KASSERTSTR, "", #e, __FILE__, __LINE__))
 #else
 #define        assert(e)       (__predict_true((e)) ? (void)0 :                
    \
-                           kern_assert("", __FILE__, __LINE__, "e"))
+                           panic(__KASSERTSTR, "", "e", __FILE__, __LINE__))
 #endif
 #endif
 
@@ -197,48 +200,53 @@ tolower(int ch)
 #ifndef DIAGNOSTIC
 #define _DIAGASSERT(a) (void)0
 #ifdef lint
-#define        KASSERTMSG(e, msg)      /* NOTHING */
+#define        KASSERTMSG(e, msg, ...) /* NOTHING */
 #define        KASSERT(e)              /* NOTHING */
 #else /* !lint */
-#define        KASSERTMSG(e, msg)      ((void)0)
+#define        KASSERTMSG(e, msg, ...) ((void)0)
 #define        KASSERT(e)              ((void)0)
 #endif /* !lint */
 #else /* DIAGNOSTIC */
 #define _DIAGASSERT(a) assert(a)
-#define        KASSERTMSG(e, msg) do {         \
-       if (__predict_false(!(e)))      \
-               panic msg;              \
-       } while (/*CONSTCOND*/ 0)
+#define        KASSERTMSG(e, msg, ...)         \
+                       (__predict_true((e)) ? (void)0 :                    \
+                           panic(__KASSERTSTR msg, "diagnostic ", #e,      \
+                               __FILE__, __LINE__, ## __VA_ARGS__))
 #ifdef __STDC__
 #define        KASSERT(e)      (__predict_true((e)) ? (void)0 :                
    \
-                           kern_assert("diagnostic ", __FILE__, __LINE__, #e))
+                           panic(__KASSERTSTR, "diagnostic ", #e,          \
+                               __FILE__, __LINE__))
 #else
 #define        KASSERT(e)      (__predict_true((e)) ? (void)0 :                
    \
-                           kern_assert("diagnostic ", __FILE__, __LINE__,"e"))
+                           panic(__KASSERTSTR, "diagnostic ", "e",         \
+                               __FILE__, __LINE__))
 #endif
 #endif
 
 #ifndef DEBUG
 #ifdef lint
-#define        KDASSERTMSG(e,msg)      /* NOTHING */
+#define        KDASSERTMSG(e,msg, ...) /* NOTHING */
 #define        KDASSERT(e)             /* NOTHING */
 #else /* lint */
-#define        KDASSERTMSG(e,msg)      ((void)0)
+#define        KDASSERTMSG(e,msg, ...) ((void)0)
 #define        KDASSERT(e)             ((void)0)
 #endif /* lint */
 #else
-#define        KDASSERTMSG(e, msg) do {        \
-       if (__predict_false(!(e)))      \
-               panic msg;              \
-       } while (/*CONSTCOND*/ 0)
+#define        KDASSERTMSG(e, msg, ...)        \
+                       (__predict_true((e)) ? (void)0 :                    \
+                           panic(__KASSERTSTR msg, "debugging ", #e,       \
+                               __FILE__, __LINE__, ## __VA_ARGS__))
 #ifdef __STDC__
 #define        KDASSERT(e)     (__predict_true((e)) ? (void)0 :                
    \
-                           kern_assert("debugging ", __FILE__, __LINE__, #e))
+                           panic(__KASSERTSTR, "debugging ", #e,           \
+                               __FILE__, __LINE__))
 #else
 #define        KDASSERT(e)     (__predict_true((e)) ? (void)0 :                
    \
-                           kern_assert("debugging ", __FILE__, __LINE__, "e"))
+                           panic(__KASSERTSTR, "debugging ", "e",          \
+                               __FILE__, __LINE__))
 #endif
 #endif
+
 /*
  * XXX: For compatibility we use SMALL_RANDOM by default.
  */