Re: [Xen-devel] [PATCH 3 of 3] xen/debug: Introduce ASSERT_RUN()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 08.10.12 at 20:16, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
> This is a variant of ASSERT() which takes a predicate, a function an
> argument for the function.  It is designed for debugging in situations
> where ASSERT_PRINTK() is perhaps not powerful enough.
> 
> It will run the given function with the given argument before the BUG()
> which kills Xen.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> 
> diff -r 477ccdb9870e -r 0a1a3f35f56a xen/include/xen/lib.h
> --- a/xen/include/xen/lib.h
> +++ b/xen/include/xen/lib.h
> @@ -48,6 +48,16 @@ do {                                    
>  } while (0)
>  #endif /* assert_printk_failed */
>  
> +#ifndef assert_run_failed
> +#define assert_run_failed(p, func, arg)                         \
> +do {                                                            \
> +    printk("Assertion '%s' failed, line %d, file %s\n", p ,     \
> +                   __LINE__, __FILE__);                         \
> +    (func)((arg));                                              \

Quite a few pointless parentheses here.

> +    BUG();                                                      \
> +} while (0)
> +#endif /* assert_run_failed */
> +
>  #ifdef CONFIG_ASSERTS
>  #define ASSERT(p) \
>      do { if ( unlikely(!(p)) ) assert_failed(#p); } while (0)
> @@ -55,9 +65,15 @@ do {                                    
>  #define ASSERT_PRINTK(p, ...)                                   \
>      do { if ( unlikely(!(p)) )                                  \
>              assert_printk_failed(#p, __VA_ARGS__); } while (0)
> +
> +/* func expected to be void, taking a single void * argument */
> +#define ASSERT_RUN(p, func, arg)                                \

Since the user of the construct specifies both func and arg, I don't
see the need to specify what type they are. Nor is it meaningful
here whether the function returns void (it would at most need to
be stated - I'd consider this mostly obvious - that an eventual
return value doesn't get used).

> +    do { if ( unlikely(!(p)) )                                  \
> +            assert_run_failed(#p, func, arg); } while (0)
>  #else
>  #define ASSERT(p) do { if ( 0 && (p) ); } while (0)
>  #define ASSERT_PRINTK(p, ...) do { if ( 0 && (p) ); } while (0)
> +#define ASSERT_RUN(p, func, arg) do { if ( 0 && (p) ); } while (0)

You shall evaluate func and arg (i.e. invoke func(arg)) in the
(dead) if() body, to both avoid the need for #ifdef-s at use
sites and check type compatibility even in non-debug (or non-
assert, following your earlier patch) builds.

Jan

>  #endif
>  
>  #define ABS(_x) ({                              \




_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel