From owner-svn-src-head@FreeBSD.ORG Sat May 23 14:08:00 2009 Return-Path: Delivered-To: svn-src-head@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 62F36106566C; Sat, 23 May 2009 14:08:00 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail07.syd.optusnet.com.au (mail07.syd.optusnet.com.au [211.29.132.188]) by mx1.freebsd.org (Postfix) with ESMTP id EE4F78FC0C; Sat, 23 May 2009 14:07:59 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from besplex.bde.org (c122-107-117-19.carlnfd1.nsw.optusnet.com.au [122.107.117.19]) by mail07.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id n4NE7sqP026043 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Sun, 24 May 2009 00:07:56 +1000 Date: Sun, 24 May 2009 00:07:54 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: "Bjoern A. Zeeb" In-Reply-To: <20090523113511.N72053@maildrop.int.zabbadoz.net> Message-ID: <20090523230220.E2223@besplex.bde.org> References: <200905221810.n4MIAe4J014419@svn.freebsd.org> <20090522184846.GA34437@FreeBSD.org> <4A16F40B.4020404@freebsd.org> <20090523210556.Y826@delplex.bde.org> <20090523113511.N72053@maildrop.int.zabbadoz.net> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, Sam Leffler , Bruce Evans , src-committers@FreeBSD.org Subject: Re: svn commit: r192591 - head/sys/fs/nfsserver X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 23 May 2009 14:08:00 -0000 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 #include #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