From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH 2 of 3] xen/debug: Introduce ASSERT_PRINTK() Date: Mon, 15 Oct 2012 10:29:00 +0100 Message-ID: <507BD75C.7010905@citrix.com> References: <477ccdb9870ed68e33c5.1349720162@andrewcoop.uk.xensource.com> <507BF0C502000078000A1484@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <507BF0C502000078000A1484@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: "Keir (Xen.org)" , "xen-devel@lists.xen.org" List-Id: xen-devel@lists.xenproject.org On 15/10/12 10:17, Jan Beulich wrote: >>>> On 08.10.12 at 20:16, Andrew Cooper wrote: >> This is a variant of ASSERT() which takes a predicate, and a variable >> number of arguments which get fed to prink() before the BUG(). >> >> Signed-off-by: Andrew Cooper >> >> -- >> This does use C99 varadic macros, but given that we use other C99 >> features without #ifdef guards, I felt it not necessary to guard this as >> well. >> >> diff -r 2927e18e9a7c -r 477ccdb9870e xen/include/xen/lib.h >> --- a/xen/include/xen/lib.h >> +++ b/xen/include/xen/lib.h >> @@ -38,11 +38,26 @@ do { >> } while (0) >> #endif >> >> +#ifndef assert_printk_failed >> +#define assert_printk_failed(p, ...) \ >> +do { \ >> + printk("Assertion '%s' failed, line %d, file %s\n", p , \ >> + __LINE__, __FILE__); \ >> + printk(__VA_ARGS__); \ > The first argument here necessarily is a format string, so it > should also be enforced that way. Except for the trailing comma issue present in C99 varadic macros, which is why it is specified this way. #define COMMA(fmt, ...) printf(fmt, _VA_ARGS__); Calling COMMA("foobar") will expand to 'printf("foobar",);' leading to a syntax error. There is a GCCism which fixes this issue, but it is not portable. > Which then opens the > question whether the two printk()-s shouldn't be folded (at the > price of requiring the format string to be a literal). I would err away from that option if possible, just for flexibility sake. > > I wonder though whether we wouldn't be better off following > Linux'es WARN() et al infrastructure, rather than extending the > ASSERT() one. > > Jan Keir implied that this might like to be extended to BUG()s and WARN()s, which I am happy to do if that is the consensus. ~Andrew > >> + BUG(); \ >> +} while (0) >> +#endif /* assert_printk_failed */ >> + >> #ifdef CONFIG_ASSERTS >> #define ASSERT(p) \ >> do { if ( unlikely(!(p)) ) assert_failed(#p); } while (0) >> + >> +#define ASSERT_PRINTK(p, ...) \ >> + do { if ( unlikely(!(p)) ) \ >> + assert_printk_failed(#p, __VA_ARGS__); } while (0) >> #else >> #define ASSERT(p) do { if ( 0 && (p) ); } while (0) >> +#define ASSERT_PRINTK(p, ...) do { if ( 0 && (p) ); } while (0) >> #endif >> >> #define ABS(_x) ({ \ > > -- Andrew Cooper - Dom0 Kernel Engineer, Citrix XenServer T: +44 (0)1223 225 900, http://www.citrix.com