svn commit: r192591 - head/sys/fs/nfsserver

Bruce Evans brde at optusnet.com.au
Sat May 23 14:08:00 UTC 2009


On Sat, 23 May 2009, Bjoern A. Zeeb wrote:

>> On Fri, 22 May 2009, Sam Leffler wrote:
>> 
>>> I requested the printf identify the call site; e.g.
>>> 
>>> printf("%s: out of clientids\n", __func__);
>> 
>> That is equally cryptic _with_ grepping through the source code, and
>> much uglier.  __func__ should only be used when the function name is
>> not a literal constant (mainly in macros).
>
> Even if I am going to regret this: I strongly say "NO" here.
>
> Using __func__ in printfs helps for a lot of things. Let me tell you
> the three that mostly annoyed me over the last months, constantly
> tripping over:
>
> - people move code from one function to another and do not update all
>  the printfs. __func__ does that for you.
> - people copy and paste code and do not update the printfs and old
>  function names, sometimes entirely unrelated, even in KASSERTs, stay.
>  __func__ does not have that problem.

Great, the result is correct function names in otherwise wrong code when
people blindly copy code.

> - if I want to find function definitions and function calls using gre
>  I do not want to find 47 printfs with the function name as well.
>  __func__ does not show up in grep.

This can be considered a feature of literal names too.  It encourages
not putting 47 printfs in 1 function.

> I strongly vote for using __func__ in printfs!

If you want to leave the function name out of literal strings, this can
be done better using a macro to add it.  This has been easy to do portably
since the same time that __func__ became Standard, using C99 variadic
macros.  Here is a simple implementation.  I also degrotted KASSERT() ...

%%%
#include <stdarg.h>
#include <stdio.h>

#define	funcprintf(...) do {					\
 	printf("%s: ", __func__);				\
 	printf(__VA_ARGS__);					\
} while (0)

#define	KASSERT(exp, ...) do {					\
 	if (__predict_false(!(exp)))				\
 		funcpanic(__func__, __VA_ARGS__);		\
} while (0)

void	funcpanic(const char *, const char *, ...) __printflike(2, 3);

int
main(void)
{
 	int off;

 	funcprintf("test\n");
 	funcprintf("test %d\n", 1);
 	off = -1;
 	KASSERT(off >= 0, "negative off");
 	KASSERT(off >= 0, "negative off %d", off);
 	return (0);
}

void
funcpanic(const char *func, const char *fmt, ...)
{
 	va_list ap;

 	va_start(ap, fmt);
 	printf("panic: %s: ", func);
 	vprintf(fmt, ap);
 	printf("\n");
 	va_end(ap);
}
%%%

... For KASSERT(), this automatically prepends the function name, and it
changes the API to not require (or allow) the ugly parenthesization of the
`msg' arg.
     (Note that this arg is not visible in the macro definition above
     because C99 variadic macros have syntactic problems -- the arg
     before the variadic args must not be listed since a comma would
     be needed between it and __VA_ARGS__, but then the comma would be
     a syntax error if __VA_ARGS__ is null.  In gcc the comma can be
     annulled using special token pasting, but C99 doesn't have this.)

IIRC, the current KASSERT() API only requires this parenthesization
because portable variadic macros weren't available when it was implemented
in 1998.  This should have been fixed in ~2000 before there were so many
KASSERT()s.

IIRC, the KASSERT() API was intentionally made unlike the assert(3) API
so as to provided the caller full control over the contents of the message.
This hasn't worked well.  The explicit messages mainly make KASSERT()s
harder to read and write.  I would prefer automatic printing of the
function name (only) followed by stringization of the expression.
Variadic macros should make it easy to bypass this to get full control
over the contents of the message if required.

Here is a simple modification of the above that ignores the message (if
any) and stringizes the expression:

%%%
#define	KASSERT(exp, ...) do {					\
 	if (__predict_false(!(exp)))				\
 		funcpanic(__func__, "%s", #exp);		\
} while (0)
%%%

Bruce


More information about the svn-src-head mailing list