[PATCH v2] xen: Rework WARN_ON() to return whether a warning was triggered

Julien Grall posted 1 patch 3 years, 4 months ago
Test gitlab-ci failed
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20201218133054.7744-1-julien@xen.org
xen/include/xen/lib.h | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
[PATCH v2] xen: Rework WARN_ON() to return whether a warning was triggered
Posted by Julien Grall 3 years, 4 months ago
From: Julien Grall <jgrall@amazon.com>

So far, our implementation of WARN_ON() cannot be used in the following
situation:

if ( WARN_ON() )
    ...

This is because WARN_ON() doesn't return whether a warning has been
triggered. Such construciton can be handy if you want to print more
information and also dump the stack trace.

Therefore, rework the WARN_ON() implementation to return whether a
warning was triggered. The idea was borrowed from Linux

Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

---
    Changes in v2:
        - Rework the commit message
        - Don't use trailing underscore
        - Add Bertrand's and Juergen's reviewed-by
---
 xen/include/xen/lib.h | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
index 48429b69b8df..5841bd489c35 100644
--- a/xen/include/xen/lib.h
+++ b/xen/include/xen/lib.h
@@ -23,7 +23,13 @@
 #include <asm/bug.h>
 
 #define BUG_ON(p)  do { if (unlikely(p)) BUG();  } while (0)
-#define WARN_ON(p) do { if (unlikely(p)) WARN(); } while (0)
+#define WARN_ON(p)  ({                  \
+    bool ret_warn_on_ = (p);            \
+                                        \
+    if ( unlikely(ret_warn_on_) )       \
+        WARN();                         \
+    unlikely(ret_warn_on_);             \
+})
 
 /* All clang versions supported by Xen have _Static_assert. */
 #if defined(__clang__) || \
-- 
2.17.1


Re: [PATCH v2] xen: Rework WARN_ON() to return whether a warning was triggered
Posted by Stefano Stabellini 3 years, 4 months ago
On Fri, 18 Dec 2020, Julien Grall wrote:
> From: Julien Grall <jgrall@amazon.com>
> 
> So far, our implementation of WARN_ON() cannot be used in the following
> situation:
> 
> if ( WARN_ON() )
>     ...
> 
> This is because WARN_ON() doesn't return whether a warning has been
> triggered. Such construciton can be handy if you want to print more
> information and also dump the stack trace.
> 
> Therefore, rework the WARN_ON() implementation to return whether a
> warning was triggered. The idea was borrowed from Linux
> 
> Signed-off-by: Julien Grall <jgrall@amazon.com>
> Reviewed-by: Juergen Gross <jgross@suse.com>
> Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

Acked-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>     Changes in v2:
>         - Rework the commit message
>         - Don't use trailing underscore
>         - Add Bertrand's and Juergen's reviewed-by
> ---
>  xen/include/xen/lib.h | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
> index 48429b69b8df..5841bd489c35 100644
> --- a/xen/include/xen/lib.h
> +++ b/xen/include/xen/lib.h
> @@ -23,7 +23,13 @@
>  #include <asm/bug.h>
>  
>  #define BUG_ON(p)  do { if (unlikely(p)) BUG();  } while (0)
> -#define WARN_ON(p) do { if (unlikely(p)) WARN(); } while (0)
> +#define WARN_ON(p)  ({                  \
> +    bool ret_warn_on_ = (p);            \
> +                                        \
> +    if ( unlikely(ret_warn_on_) )       \
> +        WARN();                         \
> +    unlikely(ret_warn_on_);             \
> +})
>  
>  /* All clang versions supported by Xen have _Static_assert. */
>  #if defined(__clang__) || \
> -- 
> 2.17.1
> 

Re: [PATCH v2] xen: Rework WARN_ON() to return whether a warning was triggered
Posted by Jan Beulich 3 years, 4 months ago
On 18.12.2020 14:30, Julien Grall wrote:
> From: Julien Grall <jgrall@amazon.com>
> 
> So far, our implementation of WARN_ON() cannot be used in the following
> situation:
> 
> if ( WARN_ON() )
>     ...
> 
> This is because WARN_ON() doesn't return whether a warning has been
> triggered. Such construciton can be handy if you want to print more
> information and also dump the stack trace.
> 
> Therefore, rework the WARN_ON() implementation to return whether a
> warning was triggered. The idea was borrowed from Linux
> 
> Signed-off-by: Julien Grall <jgrall@amazon.com>
> Reviewed-by: Juergen Gross <jgross@suse.com>
> Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

In particular to clarify my prior concerns have been addressed:

Acked-by: Jan Beulich <jbeulich@suse.com>

Jan