Re: KASSERTMSG fix

[Joerg Sonnenberger <joerg%britannica.bec.de@localhost>]

On Wed, Sep 07, 2011 at 05:43:14PM +0200, Jean-Yves Migeon wrote:
> On 07.09.2011 17:02, Joerg Sonnenberger wrote:
> > On Wed, Sep 07, 2011 at 04:54:05PM +0200, Jean-Yves Migeon wrote:
> >> The attached patch takes care of this; it does not really "append" the
> >> msg to the panic string (so it won't be available via panicstr like
> >> before), but now KASSERTMSG() will print the additional info.
> > 
> > I'm not sure I like the approach. I would prefer to introduce a second
> > panic() function with explicit file/line number / function argument.
> > panic() itself could be replaced by a macro calling that new verbose
> > panic function, maybe under an option.
> 
> Hmm. This is generally done via the fmt string of panic(9). Someone
> could also ask to have explicit timestamping and function name, the
> "verbose panic" prototype will become rapidly ugly...

Timestamping can be done in panic itself. For the rest, it is more a
question of consistency and ease of use. I haven't tried building a
kernel to see what the size impact of always including them would be.

> > I think it is time for KASSERTMSG and friends to just adopt C99 variadic
> > macros with the associated dropping of a pair of (). That involves more
> > code churn though.
> 
> I did not propose this for this reason: I can fix all the KASSERTMSG()
> uses in src, but not those outside the tree. And I expect that most
> compile time warnings here won't make much sense once the macro gets
> changed.

I don't think the compiler warnings will degrade much, even with GCC.
I don't think there is enough external code to justify worrying about
it.

> This can work around the limitations elegantly though (provided all call
> sites are fixed):
> 
> #define KASSERTMSG(e, fmt, ...) do {            \
>   panic(                                        \
>     "kernel diag assert \"%s\" failed: file \"%s\", line %d; " fmt, \
>     #e, __FILE__, __LINE__, ## __VA_ARGS__);    \
> } while(0)

That assumes that fmt is a string literal. I don't think it is easier
than:

panic_verbose(__FILE__, __LINE__, #e, fmt, ##__VA_ARGS__);

and it would allow panic to be turned into the verbose form easily.

> But do all compilers used with NetBSD support variadic macros?

The kernel is C99, any compiler not supporting that is irrelevant.

Joerg